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)

3 Branch
enhancement
Not set
normal

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?
Andrew, any thoughts on this bug as well?
Flags: needinfo?(ahalberstadt)
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)
If we set up a mach command for this at a later date, getting the task to use it is also trivial.
Assignee: nobody → standard8
Per Andrew's comments, I'm running with this first and we'll consider a mach command later.
No longer depends on: 1379118
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 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 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 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 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+
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
Depends on: 1380801
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: