Closed Bug 1275423 Opened 8 years ago Closed 7 years ago

Unit test TestJSONWebTokenUtils > testDSAGeneration fails locally

Categories

(Firefox for Android Graveyard :: Testing, defect)

All
Android
defect
Not set
normal

Tracking

(firefox56 fixed, firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: mcomella, Assigned: Grisha)

References

Details

Attachments

(1 file)

with:

org.mozilla.gecko.browserid.test.TestJSONWebTokenUtils > testDSAGeneration FAILED
    org.junit.ComparisonFailure at TestJSONWebTokenUtils.java:125
It looks like we're trying to compare two strings of JSON with assertEquals [1] but the parameters in the string are swapped. Oops!

https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/background/junit4/src/org/mozilla/gecko/browserid/test/TestJSONWebTokenUtils.java#125
Assignee: nobody → michael.l.comella
I forced that assertion to pass locally & the assertions after following. Notably, they're commented with [1]:
 // Really(!) brittle tests below.  The DSA signature algorithm is not deterministic, so we can't test the actual signature.

I'm not entirely certain what's going on here or why it's passing in automation but I don't think it's important enough to put more time into at the moment.

[1]: https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/background/junit4/src/org/mozilla/gecko/browserid/test/TestJSONWebTokenUtils.java#
Assignee: michael.l.comella → nobody
This is now happening after Bug 1376306.  All that needs to happen is the components in the JSON object need to be compared, not the strings.

grisha, could you follow-up with the trivial fix?  I don't want Bug 1376306 to get backed out!
Flags: needinfo?(gkruglov)
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
Flags: needinfo?(gkruglov)
Pushed patch makes tests happy locally. Let's see how it does on try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3932195739f3
Comment on attachment 8892709 [details]
Bug 1275423 - Update failing DSA signature tests

https://reviewboard.mozilla.org/r/163696/#review169100

The generated DSA string bakes in the JSON of the key, so if the JSON changes, that will change too.

This is safe because we now _require_ Java >1.8, which seems to be consistent with how the JSON displays.  That could change across OS's, Java versions, etc, but let's just fix automation for now.

Thanks for taking this for me, Grisha!
Attachment #8892709 - Flags: review?(nalexander) → review+
Unit tests pass in automation. Landing!
Also, thanks for clarifying, Nick! I was sure to leave the "this is brittle" comment intact :)
Pushed by gkruglov@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a0899eec8a93
Update failing DSA signature tests r=nalexander
https://hg.mozilla.org/mozilla-central/rev/a0899eec8a93
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.