Closed Bug 1425929 Opened 2 years ago Closed 2 years ago

test-verify jobs should pick a virtualization appropriate to the test

Categories

(Testing :: General, enhancement, P3)

57 Branch
enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: kats, Assigned: jmaher)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

the gfx/tests/mochitest/test_acceleration.html test is part of the "gpu" subsuite of tests, and on windows, that test suite runs on a VM with gpu support [1]. However, the test-verify job does not [2][3] (it falls back to the default of "virtual"). This means that if a gpu-subsuite test runs in the test-verify job, it doesn't have the GPU available and so fails. This happened in [4] for example.

[1] https://searchfox.org/mozilla-central/rev/b1e0ae2573e88391a7ed92407568be77994c9317/taskcluster/ci/test/mochitest.yml#256
[2] https://searchfox.org/mozilla-central/rev/b1e0ae2573e88391a7ed92407568be77994c9317/taskcluster/ci/test/misc.yml#55
[3] https://searchfox.org/mozilla-central/rev/b1e0ae2573e88391a7ed92407568be77994c9317/taskcluster/taskgraph/transforms/tests.py#222
[4] https://treeherder.mozilla.org/#/jobs?repo=try&revision=41aed366e234b975f5b7b0f83fb9e30827b13eb6
Thanks :kats. I was aware of this vulnerability in general, but couldn't put my finger on where this would be a practical problem -- this is a good example.

I may need to think a little about how to solve this...
Assignee: nobody → gbrown
Priority: -- → P3
this doesn't work on exceptions like:
* win10 reftests (hardware)
* xlarge vs large (we default to xlarge for test-verify)
* win10-qr specific gpu needs

while this isn't perfect it might be a starting point to come up with better ideas... :)
Assignee: gbrown → jmaher
Status: NEW → ASSIGNED
Attachment #8966757 - Flags: review?(gbrown)
Comment on attachment 8966757 [details] [diff] [review]
find tests that need gpu and run them on gpu instances

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

As you say, this doesn't solve the whole problem, but it certainly helps, and I don't have a better idea.

::: taskcluster/taskgraph/transforms/tests.py
@@ +799,5 @@
>          file_patterns = ['**/test_*', '**/browser_*', '**/crashtest*/**',
>                            'js/src/test/test/', 'js/src/test/non262/', 'js/src/test/test262/']
> +    elif type == 'test-verify-gpu-e10s':
> +        file_patterns = ['**/*webgl*/**/test_*', '**/dom/canvas/**/test_*', '**/gfx/tests/**/test_*',
> +                         '**/devtools/canvasdebugger/**/browser_*', '**/reftest*/**']

Wow, that's ugly, and I have no idea how accurate it is.

@@ +809,5 @@
>          for path in changed_files:
>              if mozpackmatch(path, pattern):
> +                gpu = False
> +                if type == 'test-verify-e10s':
> +                    gpu_dirs = ['dom/canvas', 'gfx/tests', 'devtools/canvasdebugger', 'webgl']

Why are these different from the file_patterns? No reftest?

::: testing/mozharness/mozharness/mozilla/testing/verify_tools.py
@@ +23,5 @@
>        "help": "Run additional verification on modified tests."
>        }],
> +    [["--gpu-required"],
> +     {"action": "store_true",
> +      "dest": "gpuRequired",

nit: gpu_required (or maybe just gpu?)

@@ +65,5 @@
>                  self.info("Verification updated with manifest %s" % path)
>  
>          ref_manifests = [
> +            # NOTE: win10 reftests currently require hardware, so this could be confusing
> +            (os.path.join(dirs['abs_reftest_dir'], 'tests', 'layout', 'reftests', 'reftest.list'), 'reftest', 'gpu'), #gpu

I'm not sure the #gpu comments add value.
Attachment #8966757 - Flags: review?(gbrown) → review+
thanks for the review, I know the gpu tests file patterns are ugly- as far as I know we are not changing what is defined as gpu at all- I think it is a rare exception that a new file pattern would show up.

Regarding the other comment as to why the file patterns don't match.  I added code comments to address the specifics, but in short the file_patterns for regular test-verify will pick up tests that are gpu specific.  It will not pick up 'reftest', so we don't need that.  the file_patterns need to be more specific (i.e. test_*), but once we have the list of matched files to the file_patterns, we can filter that down by directory name.

I have addressed the other nits as well as inclusive schedules!
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/88b650567a7f
test-verify jobs should pick a virtualization appropriate to the test. r=gbrown
https://hg.mozilla.org/mozilla-central/rev/88b650567a7f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.