Closed Bug 1003417 Opened 10 years ago Closed 7 years ago

Run mozbase unittests outside of 'make check'

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox54 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: dminor, Assigned: ahal)

References

(Blocks 2 open bugs)

Details

Attachments

(5 files)

Bug 1003417 - Add ability to run subsuites to |mach python-test|,
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1003417 - Add a 'mozbase' subsuite to python unittests on linux,
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1003417 - Add task for running mozbase python tests on linux,
58 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1003417 - Include testing/mozbase/moz.build from root moz.build file,
59 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1003417 - Use 'ip addr show' instead of 'ifconfig' for moznetwork tests,
59 bytes, text/x-review-board-request
ted
: review+
Details 
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.
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.
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.
Blocks: 1210759
I've made some progress towards this recently, see https://bugzilla.mozilla.org/show_bug.cgi?id=1210759#c6 for more details.
Depends on: 1320194
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.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Summary: Only run subset of mozbase unittests in 'make check' → Run mozbase unittests outside of 'make check'
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.
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.
Actually I guess that's how the image tasks work.. maybe I'm wrong.
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?
Blocks: 1335873
(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 on attachment 8816593 [details]
Bug 1003417 - Add task for running mozbase python tests on linux,

https://reviewboard.mozilla.org/r/97280/#review110758
Attachment #8816593 - Flags: review?(dustin) → review+
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.
Attachment #8816591 - Flags: review?(ted) → review+
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.
Attachment #8833460 - Flags: review?(ted) → review+
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.
Attachment #8816592 - Flags: review?(ted) → review+
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.
Attachment #8833461 - Flags: review?(ted) → review-
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 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.
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 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.
Attachment #8833461 - Flags: review?(ted) → review+
Pushed by ahalberstadt@mozilla.com:
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
Blocks: 1340162
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: