Page MenuHomePhabricator

Jenkins: JSHint jobs should compare against master or parent revision (instead of the previous tested patch)
Closed, ResolvedPublic

Description

It seems that the mediawiki-core-jslint test compares the current results against a previous "successful" run's results in order to ignore warnings. But it does not take into account that the previous run may have been on a different branch. This is not particularly useful.

In https://integration.mediawiki.org/ci/job/mediawiki-core-jslint/3455/checkstyleResult/, it compared a backport on the wmf/1.21wmf11 branch with a previous test against master, and seems to have complained about issues that had been fixed in master since wmf11 was branched.


Version: wmf-deployment
Severity: enhancement

Details

Reference
bz46066

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 22 2014, 1:13 AM
bzimport set Reference to bz46066.

This is a general problem with how Zuul and Jenkins don't work well together. They're based on different concepts.

However you should just ignore those relative results and look at the absolute results instead.

In JSHint there are two categories:

  • Warnings enabled by default we disabled and warnings disabled by default that we didn't enable ("normal")
  • Warnings we have enabled ("high")

Most reporters (including the command-line api, text-editor api and jshint.com interface) only report the high priority errors.

Although we include everything in the checkstyleResult[1], only the high priority warnings are counted towards marking the job SUCCESS or FAILURE.

MediaWiki core has 0 failures in master, so any failure is a new failure.

So when you open the report you should look at the tab "High priority"[2].

Previously we outputted them in raw text (the command-line api) which only showed the high priority errors and did not do any kind of comparison.

Now that we're displaying them visually and organised with Checkstyle, we also show ignored warnings in the "normal" category[1]. And it also does comparison against the previous run.

This last feature (comparison) is useless in our case, but it can't be disabled unfortunately. So you're best just ignoring it.

Nobody asked for that feature, it just came along with visualising the data with Checkstyle, so changing this to an "enhancement". It is confusing, but since nobody wanted this feature I'm not going to prioritise fixing it.

[1] We include ignored errors in a separate category so that they aren't displayed mixed between the others. We don't hide them completely so that they're still available for those interested in seeing what other settings we can potentially enable again in the future (e.g. if you see 0 errors for "use single quotes" in the ignored category, it means it is safe to enable that setting).

[2] https://integration.mediawiki.org/ci/job/mediawiki-core-jslint/3455/checkstyleResult/HIGH/?#details

Related URL: https://gerrit.wikimedia.org/r/58330 (Gerrit Change I29218fd9786c63eb60617ea66f8637fcd97e9937)

Marking as fixed, we no longer use the checkstyleResult.