Puppeteer jobs (cdp, bidi) are not marked as fail when exit code is not 0
Categories
(Remote Protocol :: Agent, defect, P1)
Tracking
(firefox122 fixed)
Tracking | Status | |
---|---|---|
firefox122 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
(Whiteboard: [webdriver:m9])
Attachments
(1 file)
I just saw this with the following try push:
https://treeherder.mozilla.org/jobs?repo=try&revision=28886fbf49e3b81f84cb8fb67173f4d638ec1cd7&searchStr=pup
All the cdp
and bidi
jobs are green but shouldn't given that all of them exited with an exit code not 0
.
I had another look at mozilla-central:
https://treeherder.mozilla.org/jobs?repo=mozilla-central&searchStr=pup&revision=3932e854ab7c289dfe94998ffde45c19f12b99f1
While the cdp
job is marked as failing it's not the case for wd
, and that one fails as well with:
task 2023-11-20T12:00:55.491Z] PID 711 | ❌ [test] Failed with exit status 1
[task 2023-11-20T12:00:55.501Z] PID 711 | ❌ [test:firefox:bidi] Failed with exit status 1
That means we do no longer correctly report the test status.
Andrew or Joel, has something changed which could have caused this issue? Or may this be a Treeherder issue?
Comment 1•10 months ago
|
||
[task 2023-11-20T16:09:57.080Z] npm exited with code 1
[taskcluster 2023-11-20 16:09:57.676Z] === Task Finished ===
[taskcluster 2023-11-20 16:09:58.032Z] Successful task run with exit code: 0 completed in 265.975 seconds
Npm exited 1, but the task got a 0. So this is a bug in the whatever is invoking the npm
command (mach I guess?). You need to make sure whatever invokes npm
exits non-zero so it propagates to the task return code.
Assignee | ||
Comment 2•10 months ago
|
||
Oh Interesting. So that is then indeed an issue, which exists a bit longer. I was able to also see it for a job from November 13th. Lets see if we can easily trigger this error when making an invalid invocation of npm
. At least it's good to know that we know now where it is, and sorry that I missed the final 0
exist code.
Assignee | ||
Comment 3•10 months ago
|
||
Just some observations:
- It's not always not failing as this
wd
jobs demonstrates. Here a test is failing and the job is not exiting with0
. - But other
wd
jobs on this try build do not fail even through a test is failing and requires an update to TestExpectations. - I can replicate this issue pretty easily locally when I run Puppeteer tests with having only one test enabled by using
.only
.
Here an excerpt of the log for 3:
0:04.25 pid:38420 Finished ["firefox","headless","webDriverBiDi"]
0:04.25 pid:38420 Test run matches expectations but the number of discovered tests is too low (expected: 1003, actual: 1).
0:04.26 pid:38420
0:04.26 pid:38420 ❌ [test] exited with exit code 1.
0:04.26 pid:38420 ❌ 1 script failed.
0:04.27 pid:38420
0:04.27 pid:38420 ❌ [test:firefox:bidi] exited with exit code 1.
0:04.27 pid:38420 ❌ 1 script failed.
0:04.27 SUITE_END
puppeteer-tests
~~~~~~~~~~~~~~~
Ran 1 checks (1 tests)
Expected results: 1
Unexpected results: 0
OK
Assignee | ||
Comment 4•10 months ago
|
||
I haven't had the time to verify that yet but maybe this could be a regression from the work on bug 1860170?
![]() |
||
Comment 5•10 months ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #4)
I haven't had the time to verify that yet but maybe this could be a regression from the work on bug 1860170?
I think my work did not change the relevant behaviour; actually, it looks like treating non-zero exit as success in this case may be intentional?
https://hg.mozilla.org/mozilla-central/rev/8c4b211cb72e#l1.148
Happy to have another look if you disagree.
Assignee | ||
Comment 6•10 months ago
|
||
Oh wow. This is bad. I've never taken a look at this part of the code and as such didn't notice this special case. I'm fairly sure that we should not do that anymore, and especially not with the WebDriver BiDi as protocol.
Here a try build. Lets see if it really works these days:
https://treeherder.mozilla.org/jobs?repo=try&revision=d29fbda23bff8b4a5458c184481d6ab46003e865
Assignee | ||
Comment 7•10 months ago
|
||
Updated•10 months ago
|
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3a864838360f [puppeteer] Always fail Puppeteer job when something is broken. r=webdriver-reviewers,Sasha
Comment 9•10 months ago
|
||
bugherder |
Assignee | ||
Updated•10 months ago
|
Description
•