1407178 - Enable browser_webconsole_certificate_messages.js in the new console

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1407178 - Enable browser_webconsole_certificate_messages.js in the new console; .

Review commit: https://reviewboard.mozilla.org/r/187948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/187948/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8916914 [details]
Bug 1407178 - Enable browser_webconsole_certificate_messages.js in the new console; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187948/diff/1-2/

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8916914 [details]
Bug 1407178 - Enable browser_webconsole_certificate_messages.js in the new console; .

https://reviewboard.mozilla.org/r/187948/#review193554

Looks good!

R+ (green on Win10)

Honza

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f3dd84d4d22
Enable browser_webconsole_certificate_messages.js in the new console; r=Honza.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for frequently failing enabled test:

https://hg.mozilla.org/integration/autoland/rev/a97a53dfbaeb7373969c73a39f6e4c004577aa23

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2f3dd84d4d228a6a52f47164eabd1298ed26ebe9&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=136223822&repo=autoland
> devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_certificate_messages.js | There is a warning message for SHA-1

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

I can reproduce.
The thing is, the original test we copied fails to for `./mach test devtools/client/webconsole/test/browser_webconsole_certificate_messages.js --verify`

I think this has to do with how we don't want to show security messages again if you reload the page.
Should I find a way to make the test green when using test-verify, or is there some way to bypass test-verify since it hits a feature ?

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Geoff, can you answer the questions in comment 6, please?

From the sheriffing point of view, having the test running in test-verify without special behavior would be the best. Is it possible to run a cleanup function after the test like clearing the cache so the issue goes away?

Comment 8

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I can't think of a way to bypass test-verify (other than disabling the test, obviously not what you want here!).

It would be best to find a way to make the test green with test-verify, otherwise it will fail TV the next time the test is modified, and we'll have another conversation like this. :)

It looks like your test is failing test-verify on the first step, which runs the test with --repeat. That will cause trouble for tests that rely on an initial state that is not restored at the end of the test: Your security message hypothesis sounds right. It is tempting to argue, "just don't run the test with --repeat", but it leaves your test vulnerable to the possibility that someone will add a new test, run earlier in the manifest, that changes that state and causes your test to fail. Better to "fix" your test.

That said, I think the argument "This test failed verify before, and I think I am improving the test" is a reasonable argument for proceeding with landing with test-verify failing, if you really want to.

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8916914 [details]
Bug 1407178 - Enable browser_webconsole_certificate_messages.js in the new console; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/187948/diff/2-3/

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8916914 [details]
Bug 1407178 - Enable browser_webconsole_certificate_messages.js in the new console; .

Clearing the review flag since things changed a bit since the backout

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8916914 [details]
Bug 1407178 - Enable browser_webconsole_certificate_messages.js in the new console; .

https://reviewboard.mozilla.org/r/187948/#review203726

Looks reasonable
Honza

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4619d49c91f5
Enable browser_webconsole_certificate_messages.js in the new console; r=Honza.

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4619d49c91f5

Comment 14

[Gerry Chang [:gchang]]

Mass won't fix for 58. Let it ride the train.