Page MenuHomePhabricator

[upstream] Jenkins: jsduck test is sometimes passing when the build contains warnings
Closed, ResolvedPublic

Description

Excerpted from https://integration.wikimedia.org/ci/job/mediawiki-core-jsduck/2677/console:

04:26:40 [mediawiki-core-jsduck] $ /bin/bash /tmp/hudson3159025049392134101.sh
04:26:40 Running JSDuck with /srv/ssd/jenkins-slave/workspace/mediawiki-core-jsduck/maintenance/jsduck/config.json...
04:26:41 Warning: /srv/ssd/jenkins-slave/workspace/mediawiki-core-jsduck/resources/mediawiki/mediawiki.js:1352: @inheritdoc #newStyleTag - member not found
04:26:41 Warning: /srv/ssd/jenkins-slave/workspace/mediawiki-core-jsduck/resources/mediawiki/mediawiki.Title.js:301: Unknown type HTMLImageElement|jQuery
04:26:42 Build step 'Execute shell' marked build as failure
04:26:42 Finished: FAILURE

Neither warning is related to my change, as far as I can tell.

Please be more careful about making jobs voting; an unwarranted -1 is demoralizing and undermines reviewer confidence in the patch.


Version: wmf-deployment
Severity: minor
See Also:
https://github.com/senchalabs/jsduck/issues/525

Details

Reference
bz55668

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:30 AM
bzimport set Reference to bz55668.
bzimport added a subscriber: Unknown Object (MLST).

I'm rephrasing this bug to what is the real bug, which incidentally is contradictory to your premise.

Jobs should fail regardless of whether files that seemingly caused it were altered in said patch set. Master is always passing (enforced by our gate), and if a patchset made it fail, that needs to be addressed first. If someone bypassed the gate or if Jenkins was changed without being backwards compatible either that change needs to be rollbacked or someone is presumably already working on hotfixing the (whether or not anticipated) breakage.

I'm not necessarily saying jobs absolutely must fail if the error seems to have been there already without this patchset. Merely that the problem it is causing now is actually something very different, and in general I believe at at this point anything we'd do to "ignore" errors will do more harm than good (e.g. changing jshintrc can seriously alter and break things even though the error message will not appear to originate from that file, and changes might fix one error and introduce another).

Worked around by making the jsduck Jenkins jobs to grep for 'Warning' and exit non zero in such a case: https://gerrit.wikimedia.org/r/#/c/113033/

Lowering priority (we have a workaround) and marking bug as depending on upstream.

Upstream bug is https://github.com/senchalabs/jsduck/issues/525

Change 174619 had a related patch set uploaded by Krinkle:
jsduck: Remove redundant hack that searched jsduck.log for "Warning"

https://gerrit.wikimedia.org/r/174619

Change 174619 merged by jenkins-bot:
jsduck: Remove redundant hack that searched jsduck.log for "Warning"

https://gerrit.wikimedia.org/r/174619

Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).