Closed
Bug 1353458
Opened 8 years ago
Closed 8 years ago
security/manager/ssl/tests/unit/test_cert_blocklist.js depends on hashtable enumeration order
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: dbaron, Unassigned)
References
Details
Attachments
(1 file)
I was trying to land bug 1352889, one of whose effects is to change the hashing used in PLDHashTable. This changes hashtable enumeration order.
This change led to a single test failure, in security/manager/ssl/tests/unit/test_cert_blocklist.js . This test failure is easier to understand if I change kTruncateLength from 128 to 4096 here:
https://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/testing/modules/Assert.jsm#54
The failure (with the larger kTruncateLength) is:
0:02.25 TEST_STATUS: Thread-1 FAIL revocations.txt should be as expected - "# Auto generated contents. Do not edit.\\nMCIxIDAeBgNVBAMMF0Fub3RoZXIgVGVzdCBFbmQtZW50aXR5\\n\\tVCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=\\nMBgxFjAUBgNVBAMMDU90aGVyIHRlc3QgQ0E=\\n exJUIJpq50jgqOwQluhVrAzTF74=\\nMBIxEDAOBgNVBAMMB1Rlc3QgQ0E=\\n BVio/iQ21GCi2iUven8oJ/gae74=\\nYW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy\\n YW5vdGhlciBzZXJpYWwu\\n c2VyaWFsMi4=" == "# Auto generated contents. Do not edit.\\nMCIxIDAeBgNVBAMMF0Fub3RoZXIgVGVzdCBFbmQtZW50aXR5\\n\\tVCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=\\nMBIxEDAOBgNVBAMMB1Rlc3QgQ0E=\\n BVio/iQ21GCi2iUven8oJ/gae74=\\nMBgxFjAUBgNVBAMMDU90aGVyIHRlc3QgQ0E=\\n exJUIJpq50jgqOwQluhVrAzTF74=\\nYW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy\\n YW5vdGhlciBzZXJpYWwu\\n c2VyaWFsMi4="
/home/dbaron/builds/mozilla-central/obj/firefox-debugopt/_tests/xpcshell/security/manager/ssl/tests/unit/test_cert_blocklist.js:check_revocations_txt_contents:208
/home/dbaron/builds/mozilla-central/obj/firefox-debugopt/_tests/xpcshell/security/manager/ssl/tests/unit/test_cert_blocklist.js:run_test/<:303
/home/dbaron/builds/mozilla-central/mozilla/testing/xpcshell/head.js:_run_next_test:1569
/home/dbaron/builds/mozilla-central/mozilla/testing/xpcshell/head.js:run:703
/home/dbaron/builds/mozilla-central/mozilla/testing/xpcshell/head.js:_do_main:211
/home/dbaron/builds/mozilla-central/mozilla/testing/xpcshell/head.js:_execute_test:546
-e:null:1
0:02.25 LOG: Thread-1 INFO exiting test
0:02.25 LOG: Thread-1 ERROR 2147500036
This shows the lines in the expected output being reordered.
What's the best way to proceed here? Is it actually important that the hashtable enumeration order remain unchanged, or does the test just happen to depend on it? If so, is there a better way to fix the test than to just change the test expectation?
(Note that I may need to do this *again* for bug 1352890.)
Flags: needinfo?(mgoodwin)
Updated•8 years ago
|
Whiteboard: [qf]
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(dkeeler)
There is some structure to the file that we're testing there, but we can ensure that it is as expected without depending on the hashtable enumeration order. As far as I understand, these are the requirements we need to ensure:
1. The file starts with "# Auto generated contents. Do not edit."
2. The file has four sets of related lines:
MCIxIDAeBgNVBAMMF0Fub3RoZXIgVGVzdCBFbmQtZW50aXR5
\tVCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=
MBIxEDAOBgNVBAMMB1Rlc3QgQ0E=
BVio/iQ21GCi2iUven8oJ/gae74=
MBgxFjAUBgNVBAMMDU90aGVyIHRlc3QgQ0E=
exJUIJpq50jgqOwQluhVrAzTF74=
YW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy
YW5vdGhlciBzZXJpYWwu
c2VyaWFsMi4=
where these sets may appear in any order, but the contents of each set should remain together (that is, two sets shouldn't be interleaved). For the last set in this list (the one that has two indented lines after the main line), the order of the indented lines shouldn't matter either (but to be clear, the heading line of each set should always be first).
3. The file shouldn't contain anything else.
Let me know if I should elaborate on anything here. If that's enough detail to fix this test with respect to bug 1352889 and if you have the time, feel free to write a patch - I can review it. If not, I'll find someone to address this as soon as I can.
Flags: needinfo?(dkeeler)
Updated•8 years ago
|
Whiteboard: [qf] → [qf-]
Updated•8 years ago
|
Flags: needinfo?(mgoodwin)
Reporter | ||
Comment 2•8 years ago
|
||
Comment hidden (mozreview-request) |
Reporter | ||
Comment 4•8 years ago
|
||
Er, the lint opt source-test-mozlint-eslint test actually caught something, so revised patch coming...
Comment hidden (mozreview-request) |
Reporter | ||
Comment 6•8 years ago
|
||
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8864344 [details]
Bug 1353458 - Make test_cert_blocklist more flexible about order of lines in revocations.txt.
https://reviewboard.mozilla.org/r/135990/#review139284
Nice - I like this solution. Thank you for taking the time to do this.
Attachment #8864344 -
Flags: review?(dkeeler) → review+
Reporter | ||
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4225b0990255df9f75157a74c249878a2f47103
Bug 1353458 - Make test_cert_blocklist more flexible about order of lines in revocations.txt. r=keeler
Comment 9•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•3 years ago
|
Performance Impact: --- → -
Whiteboard: [qf-]
You need to log in
before you can comment on or make changes to this bug.
Description
•