Closed
Bug 1405240
Opened 7 years ago
Closed 7 years ago
Error message assertions in test_cookie.js
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: majorpetya, Assigned: majorpetya)
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36 Steps to reproduce: Changes made to the error message assertions in test_cookie.js, for example: Assert.throws( () => cookie.add({name: "name", value: invalidType}), "Foobar"); do not take any effect, and the tests will keep on passing. Actual results: The tests passed even though the returned error message was different from the expectation. Expected results: The tests should have failed.
Assignee | ||
Updated•7 years ago
|
Component: Untriaged → Marionette
Product: Firefox → Testing
Assignee | ||
Comment 1•7 years ago
|
||
To run the test, execute: ./mach test testing/marionette/test_cookie.js
Assignee | ||
Comment 2•7 years ago
|
||
I will try to investigate this issue in the next few days.
Updated•7 years ago
|
Assignee: nobody → majorpetya
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8914798 [details] Bug 1405240 - Fix test_cookie.js unit test assertions https://reviewboard.mozilla.org/r/186076/#review191072 ::: testing/marionette/cookie.js:116 (Diff revision 1) > > if (restrictToHost) { > if (newCookie.domain !== restrictToHost) { > throw new InvalidCookieDomainError( > `Cookies may only be set ` + > - ` for the current domain (${restrictToHost})`); > + `for the current domain (${restrictToHost})`); This change was part of Bug 1402978 already, hopefully merge conflict resolution in mercurial will take care of this at the merge.
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8914798 [details] Bug 1405240 - Fix test_cookie.js unit test assertions https://reviewboard.mozilla.org/r/186076/#review191104 Good find! If you want to continue contributing you might want to check the other testing/marionette/test_*.js files if they also use strings. If you plan to submit more patches to Gecko, I would happily request commit access level 1 for you so that you can trigger your own try runs in the Mozilla CI called Treeherder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae2f39b4ff11&group_state=expanded
Attachment #8914798 -
Flags: review?(ato) → review+
Comment 6•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 8534a2a4b5fe -d 8129c4382245: rebasing 423843:8534a2a4b5fe "Bug 1405240 - Fix test_cookie.js unit test assertions r=ato" (tip) merging testing/marionette/cookie.js merging testing/marionette/test_cookie.js warning: conflicts while merging testing/marionette/test_cookie.js! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment 7•7 years ago
|
||
It looks like Mercurial is not clever enough to resolve the conflict itself. I’m pretty sure git would be able to in this case… Do you mind rebasing once your other change hits mozilla-central, and I will re-land this?
Flags: needinfo?(majorpetya)
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8914798 [details] Bug 1405240 - Fix test_cookie.js unit test assertions https://reviewboard.mozilla.org/r/186076/#review191104 I went through the output of `grep -nr 'Assert.throws' -A 2 testing/marionette` and couldn't spot any other occurrences. With regards to access: I'm happy to have access to try builds, but wouldn't want to have commit rights just yet. I'm very much a rookie on mercurial and probably could learn lot more there. // On the other hand I can barely navigate through MozReview (had to read the manual to reply :)), but I'm sure there are more excellent dev resources that can help me with try builds too.
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
(In reply to majorpetya from comment #8) > With regards to access: I'm happy to have access to try builds, > but wouldn't want to have commit rights just yet. I'm very much a > rookie on mercurial and probably could learn lot more there. I have filed https://bugzil.la/1405668 to request level 1 access for you. Note that there are a few steps you need to take that are outlined in the guidelines. With regards to version control, it is also possible to use git for Gecko development. I do this myself. You can see the workflow tutorial for how to: https://github.com/glandium/git-cinnabar/wiki/Mozilla:-A-git-workflow-for-Gecko-development If you have any questions I’m known as ato on the irc.mozilla.org server. Marionette development is mostly discussed in the #ateam channel. > // On the other hand I can barely navigate through MozReview (had > to read the manual to reply :)), but I'm sure there are more > excellent dev resources that can help me with try builds too. mozreview has a terrible UI and I’m informed it will be replaced “soon”. Whether that replacement is any better I don’t know. The other alternative is to upload your .patch files directly to the bug.
Comment 11•7 years ago
|
||
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2b4e006bc55a Fix test_cookie.js unit test assertions r=ato
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(majorpetya)
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b4e006bc55a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•7 years ago
|
Attachment #8914798 -
Flags: review?(hskupin)
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•