1003417 - Run mozbase unittests outside of 'make check'

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Dan Minor [:dminor]]

Mozbase unit tests are currently disabled in 'make check' in bug 963885 due to intermittent failures in the mozdevice tests.

Work is underway in bug 966441 to run these tests from the test package, but since the build system uses some of mozbase, we should still run some of these tests as part of the build. We need to identify the subset of tests are necessary and only run these.

I would guess we should actually run these prior to starting the build, rather than as part of 'make check' after the build is done.

Comment 1

[(not currently active) Ted Mielczarek]

We talked about moving the build system unittests to run before the build (probably after configure, since that sets up the virtualenv). We could do that separately though.

Comment 2

[Phil Ringnalda (:philor)]

Bug 963885 didn't disable mozbase tests, it disabled the mozdevice subset of mozbase tests. And then ahal clobbered even that disabling while syncing mozbase from the former RoR for the last time, so we've been running even those tests which cause builds to set RETRY and retrigger an entire new set of tests every time there's a test failure since February 18th.

Comment 4

[Andrew Halberstadt [:ahal]]

I've made some progress towards this recently, see https://bugzilla.mozilla.org/show_bug.cgi?id=1210759#c6 for more details.

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1003417 - Add ability to run subsuites to |mach python-test|,

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

Comment 6

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1003417 - Add a 'mozbase' subsuite to python unittests on linux,

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

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1003417 - Add task for running mozbase python tests on linux,

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

Comment 8

[Andrew Halberstadt [:ahal]]

I disagree with the title of this bug, we shouldn't run any mozbase unittests in make check. The mozbase unittests should not be affected by changes to mozbase itself, not to the build system.

I think the argument for keeping some of them in make check was that if a mozbase is failing, then likely a lot of other things will be failing as well and therefore we don't need to run them. But this puts mozbase unittests on the critical path (all other tests get blocked on them), which hurts our end to end times.

Given that mozbase does not change frequently, the benefits of (very rarely) catching mozbase failures early do not seem to outweigh the downsides of slower e2e times on every push.

Comment 9

[(not currently active) Ted Mielczarek]

As a followup it might be interesting to make the standalone mozbase test job block execution of other test jobs, but since it will run in parallel with the build jobs it shouldn't be a long pole. I don't think that's really necessary for starting out, though.

Comment 10

[Andrew Halberstadt [:ahal]]

Yeah, that's a good idea. Although, this mozbase task is only going to run if files in testing/mozbase/** are modified. I don't think taskcluster has the ability to define conditional dependencies yet.

Comment 11

[Andrew Halberstadt [:ahal]]

Actually I guess that's how the image tasks work.. maybe I'm wrong.

Comment 12

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

The in-tree configuration could define conditional dependencies.  TC itself doesn't need to support them.  Or maybe I've misunderstood.

How is this bug going?

Comment 13

[Andrew Halberstadt [:ahal]]

(In reply to Dustin J. Mitchell [:dustin] from comment #12) 
> How is this bug going?

I've been done the code for this bug for several weeks now, but it's still blocked on bug 1320194. However, I'm confident that there won't be any more major changes coming out of bug 1320194, so I'll upload these patches and get a head start on the review process here.

Just be aware that if you want to test things out, you'll need the patch series from the other bug as well.

Comment 14

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1003417 - Include testing/mozbase/moz.build from root moz.build file,

By including the mozbase's moz.build directly in the root moz.build, we are making sure that
mozbase unittests will be found even if there is no build/objdir. This will be useful when
running mozbase tests in their own taskcluster task.

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

Comment 15

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,

The taskcluster docker image for source-check tasks does not have 'ifconfig' installed. We
could add this package, but ifconfig is more or less deprecated in favour of 'ip addr show'.
Although the formats of both commands are different, because the test pulls ip addresses out
of the output with regexes, the only change that is needed to make sure the tests still pass
is to change the command.

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

Comment 16

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816591 [details]
Bug 1003417 - Add ability to run subsuites to |mach python-test|,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97276/diff/1-2/

Comment 17

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816592 [details]
Bug 1003417 - Add a 'mozbase' subsuite to python unittests on linux,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97278/diff/1-2/

Comment 18

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816593 [details]
Bug 1003417 - Add task for running mozbase python tests on linux,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97280/diff/1-2/

Comment 19

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

Comment on attachment 8816593 [details]
Bug 1003417 - Add task for running mozbase python tests on linux,

https://reviewboard.mozilla.org/r/97280/#review110758

Comment 20

[(not currently active) Ted Mielczarek]

Comment on attachment 8816591 [details]
Bug 1003417 - Add ability to run subsuites to |mach python-test|,

https://reviewboard.mozilla.org/r/97276/#review112016

This is a nice way to do this! I like that we'll be able to migrate tests out piecemeal without screwing up local developer workflows.

Comment 21

[(not currently active) Ted Mielczarek]

Comment on attachment 8833460 [details]
Bug 1003417 - Include testing/mozbase/moz.build from root moz.build file,

https://reviewboard.mozilla.org/r/109708/#review112020

There are times I definitely wish that we hadn't used conditionals like this in moz.build. Or at least we had some way to query the full set of moz.build data without regard to conditionals.

Comment 22

[(not currently active) Ted Mielczarek]

Comment on attachment 8816592 [details]
Bug 1003417 - Add a 'mozbase' subsuite to python unittests on linux,

https://reviewboard.mozilla.org/r/97278/#review112028

The linux-only bits are unfortunate because one of the drivers of this work is switching over to the cross-compiled OS X builds, and if we don't make this work for those builds we won't actually solve the problem.

I realize that we don't really have an OS X taskcluster testing story yet. I think we might have to look into standing up buildbot jobs for Python tests on at least OS X. With all the mozharness bits in tree, and the ability to use buildbotbridge to trigger buildbot tests, the only painful part should be adding the test types to buildbot-configs, and maybe replicating some of the source-check logic in a mozharness script.

We should at least get a bug on file for that so we don't lose track of it for the mac build migration work.

Comment 23

[(not currently active) Ted Mielczarek]

Comment on attachment 8833461 [details]
Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,

https://reviewboard.mozilla.org/r/109710/#review112030

::: testing/mozbase/moznetwork/tests/test.py:35
(Diff revision 1)
>      # 0-255.0-255.0-255.0-255, note order is important here.
>      regexip = re.compile("((25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}"
>                           "(25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)")
>  
>      if mozinfo.isLinux or mozinfo.isMac or mozinfo.isBsd:
> -        # if "/sbin/ifconfig" exist, use it because it may not be in the
> +        # if "/sbin/ip" exists, use it because it may not be in the

`ip` doesn't exist on my Mac, so you'll have to make this Linux-only.

Comment 24

[Andrew Halberstadt [:ahal]]

Comment on attachment 8833461 [details]
Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,

https://reviewboard.mozilla.org/r/109710/#review112030

> `ip` doesn't exist on my Mac, so you'll have to make this Linux-only.

Ah thanks, or better yet, I'll check for one and if it doesn't exist fallback to the other.

Comment 25

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816592 [details]
Bug 1003417 - Add a 'mozbase' subsuite to python unittests on linux,

https://reviewboard.mozilla.org/r/97278/#review112028

The testing story for osx on taskcluster might be closer than we think. I think it's mostly just blocked on in-tree taskcluster config changes.. though could be wrong about that.

Comment 26

[(not currently active) Ted Mielczarek]

OK, either way let's get a bug on file to track getting Python unittests running as standalone jobs for Mac, whether that's on taskcluster-worker or via BBB or whatever. If taskcluster-worker for Mac is ready soon enough to use it, that'd be great!

Comment 27

[Andrew Halberstadt [:ahal]]

Comment on attachment 8833460 [details]
Bug 1003417 - Include testing/mozbase/moz.build from root moz.build file,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109708/diff/1-2/

Comment 28

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816591 [details]
Bug 1003417 - Add ability to run subsuites to |mach python-test|,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97276/diff/2-3/

Comment 29

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816592 [details]
Bug 1003417 - Add a 'mozbase' subsuite to python unittests on linux,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97278/diff/2-3/

Comment 30

[Andrew Halberstadt [:ahal]]

Comment on attachment 8833461 [details]
Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109710/diff/1-2/

Comment 31

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816593 [details]
Bug 1003417 - Add task for running mozbase python tests on linux,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97280/diff/2-3/

Comment 32

[(not currently active) Ted Mielczarek]

Comment on attachment 8833461 [details]
Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,

https://reviewboard.mozilla.org/r/109710/#review112108

::: testing/mozbase/moznetwork/tests/test.py:12
(Diff revision 2)
>  import mozinfo
>  import moznetwork
>  import re
>  import subprocess
>  import unittest
> +from distutils.spawn import find_executable

We do have the `which` module vendored into the tree,  but I understand if you don't want to have to muck with the mozbase Python dependencies just to make this test work.

::: testing/mozbase/moznetwork/tests/test.py:50
(Diff revision 2)
> +    for command in commands:
> +        if find_executable(command[0]):
> +            cmd = command
> +            break
>  
> -    ps = subprocess.Popen(args, stdout=subprocess.PIPE)
> +    ps = subprocess.Popen(cmd, stdout=subprocess.PIPE)

You should fail the test if `cmd is None` here, just in case.

Comment 33

[Andrew Halberstadt [:ahal]]

Comment on attachment 8833461 [details]
Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109710/diff/2-3/

Comment 34

[Andrew Halberstadt [:ahal]]

Comment on attachment 8816593 [details]
Bug 1003417 - Add task for running mozbase python tests on linux,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/97280/diff/3-4/

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/091f6e30f5dc
Include testing/mozbase/moz.build from root moz.build file, r=ted
https://hg.mozilla.org/integration/autoland/rev/12a434138e95
Add ability to run subsuites to |mach python-test|, r=ted
https://hg.mozilla.org/integration/autoland/rev/859b523399f0
Add a 'mozbase' subsuite to python unittests on linux, r=ted
https://hg.mozilla.org/integration/autoland/rev/183a8002ba33
Use 'ip addr show' instead of 'ifconfig' for moznetwork tests, r=ted
https://hg.mozilla.org/integration/autoland/rev/22d650dc282e
Add task for running mozbase python tests on linux, r=dustin

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/091f6e30f5dc
https://hg.mozilla.org/mozilla-central/rev/12a434138e95
https://hg.mozilla.org/mozilla-central/rev/859b523399f0
https://hg.mozilla.org/mozilla-central/rev/183a8002ba33
https://hg.mozilla.org/mozilla-central/rev/22d650dc282e