1865678 - Puppeteer jobs (cdp, bidi) are not marked as fail when exit code is not 0

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

[Andrew Halberstadt [:ahal]]

[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.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just some observations:

1. It's not always not failing as this wd jobs demonstrates. Here a test is failing and the job is not exiting with 0.
2. But other wd jobs on this try build do not fail even through a test is failing and requires an update to TestExpectations.
3. 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

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

[Geoff Brown [:gbrown]]

(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.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1865678 - [puppeteer] Always fail Puppeteer job when something is broken.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a864838360f
[puppeteer] Always fail Puppeteer job when something is broken. r=webdriver-reviewers,Sasha

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3a864838360f