Closed
Bug 1379119
Opened 7 years ago
Closed 7 years ago
Add a builder to run tests for eslint-plugin-mozilla
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement)
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(3 files)
Currently the eslint-plugin-mozilla tests aren't run on automation and we should fix that.
Bug 1379118 is adding ./mach integration, I'm not sure if we'll need more than that for a test builder.
Currently to run the tests, the sequence of commands is:
cd tools/lint/eslint/eslint-plugin-mozilla
npm install
npm test
We're going to need to provide mocha somehow - maybe using a similar mechanism as eslint?
Assignee | ||
Comment 1•7 years ago
|
||
Andrew, any thoughts on this bug as well?
Flags: needinfo?(ahalberstadt)
Comment 2•7 years ago
|
||
We've come a long way in making adding new tasks easy :), and adding a mach command isn't even a prerequisite. Here are the rough steps:
1. Create a new file: taskcluster/ci/source-test/mocha.yml
2. Add that to: taskcluster/ci/source-test/kind.yml
3. Loosely copy and paste this task:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/source-test/python-tests.yml#121
4. Install dependencies in the docker image:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/docker/lint/system-setup.sh
Flags: needinfo?(ahalberstadt)
Comment 3•7 years ago
|
||
If we set up a mach command for this at a later date, getting the task to use it is also trivial.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → standard8
Assignee | ||
Comment 4•7 years ago
|
||
Per Andrew's comments, I'm running with this first and we'll consider a mach command later.
No longer depends on: 1379118
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
Example induced failure:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=734f9ee39642751ff6249b34576f7581fea4bc12&selectedJob=113074542
Example passes (with respins to check for intermittents, but I'd be very surprised if there are any):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2e429019edb2836acceef8fdf440c623334ba30&selectedJob=113084438
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8884911 [details]
Bug 1379119 - Add a builder to run the unit tests for eslint-plugin-mozilla.
https://reviewboard.mozilla.org/r/155756/#review160942
Does the tooltool archive for the regular eslint task also contain all the dependencies for eslint-plugin-mozilla? If so, I think you can follow the advice in the first issue over there instead. Then we'd have all the eslint-plugin-mozilla node_modules pre-installed in the docker image and the lock file, manifest.tt and update script would all be unnecessary (as a bonus the ES task will also run faster).
::: taskcluster/ci/source-test/mocha.yml:17
(Diff revision 1)
> + /build/tooltool.py fetch -m manifest.tt &&
> + tar xvfz eslint-plugin-mozilla.tar.gz &&
> + rm eslint-plugin-mozilla.tar.gz &&
This should happen in the docker image. Since those get cached on the workers, it'll save us from having to do this setup each time.
For the lint image, you can use this file:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/docker/lint/system-setup.sh
The `tooltool_fetch` function in there can be used like this:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/docker/centos6-build/system-setup.sh#355
Note the `"unpack": true` option.
If you don't want to hardcode the manifest.tt file in there (e.g for the update.sh script), you can include it from the Dockerfile using a special mozilla specific notation like this:
https://dxr.mozilla.org/mozilla-central/source/taskcluster/docker/lint/Dockerfile#8
Then you can just run the tooltool fetch script on it normally (and feel free to delete that unused function).
Attachment #8884911 -
Flags: review?(ahalberstadt) → review-
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8884910 [details]
Bug 1379119 - Add a mozilla specific reporter for mocha when used with eslint-plugin-mozilla to be compatible with treeherder.
https://reviewboard.mozilla.org/r/155754/#review161214
Attachment #8884910 -
Flags: review?(ahalberstadt) → review+
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8884912 [details]
Bug 1379119 - Expand test documentation for eslint-plugin-mozilla.
https://reviewboard.mozilla.org/r/155758/#review161216
Attachment #8884912 -
Flags: review?(ahalberstadt) → review+
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8884911 [details]
Bug 1379119 - Add a builder to run the unit tests for eslint-plugin-mozilla.
https://reviewboard.mozilla.org/r/155756/#review161218
::: taskcluster/ci/source-test/mocha.yml:1
(Diff revision 1)
> +epmmocha:
Fyi, this will impact the name of the task shown on treeherder as well as try syntax (though for try, it will run anytime eslint-plugin-mozilla changes anyway).
I think calling it `eslint-plugin-mozilla` might be more informative as to what this task actually is. Plus it's already in a file called mocha.yml, so the `mocha` here is superfluous.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 16•7 years ago
|
||
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8884911 [details]
Bug 1379119 - Add a builder to run the unit tests for eslint-plugin-mozilla.
https://reviewboard.mozilla.org/r/155756/#review161584
Thanks!
::: tools/lint/eslint/eslint-plugin-mozilla/update.sh:2
(Diff revision 2)
> +#!/bin/sh
> +# Script to regenerate the npm packages used for eslint-plugin-mozilla by the builders.
It might be nice to have a general mach command for uploading stuff to tooltool in the future.
::: tools/lint/eslint/eslint-plugin-mozilla/update.sh:45
(Diff revision 2)
> +# ESLint and all _external_ plugins are listed in this directory's package.json,
> +# so a regular `npm install` will install them at the specified versions.
> +# The in-tree eslint-plugin-mozilla is kept out of this tooltool archive on
> +# purpose so that it can be changed by any developer without requiring tooltool
> +# access to make changes.
This comment needs updating
Attachment #8884911 -
Flags: review?(ahalberstadt) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 21•7 years ago
|
||
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0817ffe9bc47
Add a mozilla specific reporter for mocha when used with eslint-plugin-mozilla to be compatible with treeherder. r=ahal
https://hg.mozilla.org/integration/autoland/rev/c503bc4cdc9d
Add a builder to run the unit tests for eslint-plugin-mozilla. r=ahal
https://hg.mozilla.org/integration/autoland/rev/978d4f1f9c41
Expand test documentation for eslint-plugin-mozilla. r=ahal
Comment 22•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0817ffe9bc47
https://hg.mozilla.org/mozilla-central/rev/c503bc4cdc9d
https://hg.mozilla.org/mozilla-central/rev/978d4f1f9c41
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
Product: Testing → Firefox Build System
Updated•6 years ago
|
Version: Version 3 → 3 Branch
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•