1302773 - install tox in the lint image

Status: RESOLVED FIXED

(Taskcluster Graveyard :: Docker Images, defect)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

The mozharness unit tests run

https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/source-check/python-tests.yml
        command: >
            cd /home/worker/checkouts/gecko/testing/mozharness &&
            /usr/bin/pip2 install tox &&
            /home/worker/.local/bin/tox -e py27-hg3.7

which means that on every run of this task, we're hitting pypi.python.org and re-downloading tox (not to mention then hitting it again when tox generates the py27-hg3.7 virtualenv).

At the least, we should install tox in the docker image, as we do for flake8.

Better, make this a nice mach command.

Best, install the dependencies (coverage, nose, rednose -- mercurial's already installed) in the lint image and run the test command directly, rather than via tox.

Comment 1

[Gregory Szorc [:gps]]

There are a number of Python tests that could be extracted to their own task. There's also a number of existing Python-centric tasks (like Sphinx). It might be worth creating a "python" image or some such that comes pre-installed with everything we'd ever need.

Even better would be vendoring all the packages so we don't take the chance of downloading things from the internet or using varying package versions over time due to not pinning package versions.

Comment 2

[Mark Banner (:standard8)]

This feels like it would be better under the Taskcluster product, as the work really needs to happen there I believe.

Comment 3

[Andrew Halberstadt [:ahal]]

+1 for vendoring tox.. I'd like to use it to set up the ability to run python-test with multiple versions of python so we at least have the ability to consider supporting python 3 in certain libraries.

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Not really -- there's nothing to change in taskcluster here.  It's all in-tree changes, and I think most of those are to how the tests are invoked (at least / better / best options in the first comment).  While the TC team has some expertise in the generation of tasks for Gecko, we don't have much expertise in how test harnesses work, modifying mach commands, or in mozlint specifically.

That said, two experts have now spoken up, so if someone does take this bug as a good-first-bug, I know who to needinfo :)

If we vendor tox, it'd be good to *also* vendor (or at least include in the docker image) the stuff it installs when it runs, so that we're not hitting pypi.

Comment 5

[Andrew Halberstadt [:ahal]]

(In reply to Dustin J. Mitchell [:dustin] from comment #4)
> If we vendor tox, it'd be good to *also* vendor (or at least include in the
> docker image) the stuff it installs when it runs, so that we're not hitting
> pypi.

Good point. Looks like it depends on 'py' (already vendored) and 'pluggy'. So it would just be the latter that's missing.

Also, if you want to add tox to the docker image to solve the mozharness issue in the meantime, that would be fine, and certainly easier.

Comment 6

[omomuro.m]

I will take this bug

Comment 7

[omomuro.m]

(In reply to omomuro.m from comment #6)
> I will take this bug

nevermind

Comment 8

[Swapnesh Kumar Sahoo [:swapneshks]]

Hi.. Can I help with this bug?
Which of the approaches mentioned in the description should I adopt - docker image, mach or lint?

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Let's do docker image.

Comment 10

[Swapnesh Kumar Sahoo [:swapneshks]]

I forgot to mention that I am a beginner and would be requiring some help to get started with the docker image approach. :)

I did some digging on my own though and it looks like something similar to http://searchfox.org/mozilla-central/source/taskcluster/docker/lint/Dockerfile#20 has to be done. But I am not very clear on where and how to do it..

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

I think that's a good start.  Do you have access to push to try?  This will be pretty difficult if not!

So, yes, write up a requirements.txt for tox and install and run it the same as the flake8 file (or maybe combine the two?).  Then find where the command in comment 0 is defined (somewhere under taskcluster/ci) and modify it to use the tox installed in the image, rather than installing it directly.

Comment 12

[Swapnesh Kumar Sahoo [:swapneshks]]

(In reply to Dustin J. Mitchell [:dustin] from comment #11)
> I think that's a good start.  Do you have access to push to try?  This will
> be pretty difficult if not!

Unfortunately, I don't have access to push to try, as I am very new here.. Could you vouch for me as you have reviewed my code once earlier? (https://bugzilla.mozilla.org/show_bug.cgi?id=1357802) If yes, then I'll file the required bug.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

I can -- please file the bug requesting access and I'll comment there.

Comment 14

[Swapnesh Kumar Sahoo [:swapneshks]]

Attachment: Install tox in the docker image

Let me know if the changes are appropriate and also if any other files need to be updated.

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8867352 [details] [diff] [review]
Install tox in the docker image

Review of attachment 8867352 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good (sorry for the delay.. I try to rest on weekends..)

I'll run the patch in a try job.  Now that you have level-1 access, can you try pushing this to mozreview?
  https://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview-user.html

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=42f8c5fab1ac9a7e1abb384292fc0095389e21a8

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8867352 [details] [diff] [review]
Install tox in the docker image

Swapnesh, also take a look at that try job -- it didn't work :(

Comment 18

[Swapnesh Kumar Sahoo [:swapneshks]]

(In reply to Dustin J. Mitchell [:dustin] from comment #17)
> Comment on attachment 8867352 [details] [diff] [review]
> Install tox in the docker image
> 
> Swapnesh, also take a look at that try job -- it didn't work :(

Looks like the try job didn't pick up the file tools/lint/tox/tox_requirements.txt:
https://hg.mozilla.org/try/rev/42f8c5fab1ac9a7e1abb384292fc0095389e21a8
Did I miss something while making the commit? (As the file tox_requirements.txt was an add)

Comment 19

[Dustin J. Mitchell [:dustin] (he/him)]

Oh, that's my fault, I didn't `hg add` it!

Now's your chance to try your own try job!  The message I used was "Bug 1302773: try: -b o -p lint -t none -u none"

Comment 20

[Swapnesh Kumar Sahoo [:swapneshks]]

Attachment: Bug 1302773 - Install tox in docker image in MozReview;

Review commit: https://reviewboard.mozilla.org/r/139322/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139322/

Comment 21

[Swapnesh Kumar Sahoo [:swapneshks]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed26a63092def4498d29ba3ee921c0a7a2c788f7&selectedJob=99211879

This is the try job that I tried. (I found out from IRC that I should have added the try:block in the first line)

I've also tried to push to mozreview.

As this was my first try job and first push to mozreview, do let me know if I went wrong anywhere...

Comment 22

[Dustin J. Mitchell [:dustin] (he/him)]

I made a try run with the right syntax from your mozreview request (which looks fine).
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=07d4af954631

Comment 23

[Dustin J. Mitchell [:dustin] (he/him)]

Ergh, the mozharness job didn't run there.  I've tried again.

Comment 24

[Dustin J. Mitchell [:dustin] (he/him)]

Greg, I can't figure out how to make this job run in try.  It is getting optimized away.  Halp?

Comment 25

[Dustin J. Mitchell [:dustin] (he/him)]

:swapnesh, sorry this is taking so long.  I'm going to make a fake change to mozharness to trick the job into running.

One thing to note: please don't use hg's "branch" feature -- the better feature to use is "bookmarks"
  http://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/bookmarks.html

Comment 26

[Dustin J. Mitchell [:dustin] (he/him)]

Ah, bug 1365646..

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3dd9c23364e21662a18cd8f2e3c3ede1a8a9c7e

Comment 27

[Dustin J. Mitchell [:dustin] (he/him)]

Great success!!

Comment 28

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8867791 [details]
Bug 1302773 - Install tox in docker image in MozReview;

https://reviewboard.mozilla.org/r/139322/#review143600

Comment 29

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8867791 [details]
Bug 1302773 - Install tox in docker image in MozReview;

https://reviewboard.mozilla.org/r/139322/#review143602

Sorry, clicked wrong

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e66555bcd52f
Install tox in docker image in MozReview; r=dustin

Comment 31

[Swapnesh Kumar Sahoo [:swapneshks]]

(In reply to Dustin J. Mitchell [:dustin] from comment #25)

Sorry.. I meant to reply earlier but was having internet connectivity issues.

> One thing to note: please don't use hg's "branch" feature -- the better
> feature to use is "bookmarks"
>  
> http://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/
> bookmarks.html

Ohh.. I didn't know that. Will keep it in mind in future. :)

Also, great to see the tests worked!

Comment 32

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/e66555bcd52f

Comment 33

[Dustin J. Mitchell [:dustin] (he/him)]

Thanks Swapnesh, and sorry that took me so long to land!

What's next?

Comment 34

[Swapnesh Kumar Sahoo [:swapneshks]]

(In reply to Dustin J. Mitchell [:dustin] from comment #33)
> Thanks Swapnesh, 

My pleasure!

> and sorry that took me so long to land!

No worries at all. :)

> What's next?

I'm currently working on some other moz bugs, that should not take long to fix.. So, once those are done, if there are any bugs from your side that I can work on, I'll be more than willing to help. :)