With martiniss@' awesome work on  Issue 533481 , it should be possible to turn on retries for all of the bots now. martiniss@ do you agree?

(Though I would ideally like us to move in a direction where we can disable retries for an entire test suite – and would like to do so for the GPU tests, and instead aggressively track down any newly introduced flakiness.)

This should be doable. Working on this.

Ok, looks like this is kinda bugged?

https://ci.chromium.org/swarming/task/400c463c69dfa010?server=chromium-swarm.appspot.com is a led triggered task. It runs with https://chromium-review.googlesource.com/c/chromium/tools/build/+/1232138 (which enables patch de-application on this bot) and https://crrev.com/c/1232203, which fails a test suite on the bot. The without patch run seems to run no tests, even though the test filter is correctly passed.

https://chromium-swarm.appspot.com/task?id=400c6c8b95f44910&refresh=10&show_raw=1

Anyone have an idea of what's going on?

I'm able to reproduce this locally; running:

python ../../testing/scripts/run_gpu_integration_test_as_googletest.py ../../content/test/gpu/run_gpu_integration_test.py webgl_conformance --isolated-script-test-output out.json --show-stdout --browser=debug --passthrough -v --extra-browser-args=--enable-logging=stderr --webgl-conformance-version=2.0.1 --read-abbreviated-json-results-from=../../content/test/data/gpu/webgl2_conformance_tests_output.json --isolated-script-test-filter gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_KHR_parallel_shader_compile --isolated-script-test-perf-output=perf.json


gets me:

0 tests passed in 0.1s, 0 skipped, 0 failures.

Ok, looks like a bug in run_gpu_integration_test.py.

Running `python content/test/gpu/run_gpu_integration_test.py webgl_conformance --test-filter $(python content/test/gpu/run_gpu_integration_test.py webgl_conformance -l | cat | head -1)` runs 0 tests. I'd expect it not to...

kbr@, can you take a look? 

I'm completely swamped but Kai will take a look.

This is probably as simple as trimming everything before the last "." in the test-filter arguments to this suite. It's ultimately an issue in Telemetry / typ in how the test names are reported. The part of the test name "gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest" is useless and I wish it didn't show up.

I started looking at this, forgot to mark started.
Currently thinking I'll add a new --run-test arg to run_gpu_integration_test.py which you'll use here instead of --test-filter.

--run-test=gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_KHR_parallel_shader_compile
would essentially translate to
--test-filter=WebglExtension_KHR_parallel_shader_compile

Thanks Kai for picking this up. The real issue is that src/testing/scripts/run_gpu_integration_test_as_googletest.py translates the common "--isolated-script-test-filter" argument – which is known by Chromium's recipe – into something that run_gpu_integration_test.py understands. So if a new argument is added to run_gpu_integration_test.py, the calling script will have to be changed to use it, and it'll have to handle multiple test names.

Another way to handle this might be to parse the --test-filter argument in the initial ArgumentParser and then process and re-insert the flag into rest_args_filtered.

The argument to
  run_gpu_integration_test_as_googletest.py --isolated-script-test-filter
is in the format
  group.group.TEST1::group.group.TEST2
while the argument to
  run_gpu_integration_test.py --test-filter
is a regex. run_gpu_integration_test_as_googletest.py generates a regex like this:
  (group.group.TEST1|group.group.TEST2)
but we want it to generate
  (TEST1|TEST2)
so what I've done now is add code to run_gpu_integration_test_as_google_test.py to strip the groups off during that process.

This doesn't maintain the property martiniss asserted in #7; is that a problem?

Here's a compact testcase:
  vpython testing/scripts/run_gpu_integration_test_as_googletest.py --isolated-script-test-output= --isolated-script-test-filter=gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_depth_texture::gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_draw_buffers content/test/gpu/run_gpu_integration_test.py webgl_conformance -l
This should print:
  gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_depth_texture
  gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_draw_buffers
rather than printing nothing.

I don't think it's an issue that martiniss' test case in #7 doesn't work. The recipes will be invoking run_gpu_integration_test_as_googletest.py and using the --isolated-script-test-filter command line argument with the names of the failing tests, so it's ok if just that path works.

I'm not attached to the test case that I posted; I just put it in here cause it was a short command line thing that I thought should have worked. Thanks for fixing this!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f08548e32291e294265a8e85d3b8bef69f83099a

commit f08548e32291e294265a8e85d3b8bef69f83099a
Author: Kai Ninomiya <kainino@chromium.org>
Date: Fri Sep 21 23:24:05 2018

Properly convert --isolated-script-test-filter to --test-filter

The argument to
  run_gpu_integration_test_as_googletest.py --isolated-script-test-filter
is in the format
  group.group.TEST1::group.group.TEST2
while the argument to
  run_gpu_integration_test.py --test-filter
is a regex. run_gpu_integration_test_as_googletest.py generates a regex like this:
  (group.group.TEST1|group.group.TEST2)
but we want it to generate
  (TEST1|TEST2)
.

Test:
  vpython testing/scripts/run_gpu_integration_test_as_googletest.py --isolated-script-test-output= --isolated-script-test-filter=gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_depth_texture::gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_draw_buffers content/test/gpu/run_gpu_integration_test.py webgl_conformance -l

This should print:
  gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_depth_texture
  gpu_tests.webgl_conformance_integration_test.WebGLConformanceIntegrationTest.WebglExtension_WEBGL_draw_buffers
rather than printing nothing.

Bug:  883308 
Change-Id: I5ab941705a540f431094ac97b023fee4151341b1
Reviewed-on: https://chromium-review.googlesource.com/1236292
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593373}
[modify] https://crrev.com/f08548e32291e294265a8e85d3b8bef69f83099a/testing/scripts/run_gpu_integration_test_as_googletest.py

The next step here, I think, is to turn retries back on for this bot and take out whatever dont_deapply_patch logic is no longer needed. Maybe I could take this but probably not in the next week.

I'll look at this.

Ok, I did a test of turning on the retries with the "led" tool. I ran https://ci.chromium.org/swarming/task/402b32190a4e3710?server=chromium-swarm.appspot.com. 

When it retried tests without the patch, it seems to have skipped some tests?

https://chromium-swarm.appspot.com/task?id=402b4c8a0a243810&refresh=10&show_raw=1

Is that expected? Does that mean it didn't run them for some reason? That sounds bad to me, but it seems like the code is at least filtering the tests correctly.

I think those tests should be skipped on mac - those extensions are platform specific and not supported on mac.

I'm not sure why they're running in the retry, though. They should have been skipped in with-patch, so I guess that's causing them to be retried without-patch.

That's my fault: https://chromium-review.googlesource.com/c/chromium/src/+/1232203 is the CL I'm testing everything with. So, it's failing with the patch, because it's running tests which are normally skipped, but without the patch it's not running them, because without the patch they're skipped.

So, looks like the logic is working correctly. I'll land https://crrev.com/c/1232138 and monitor the bot.

Aha, sounds good! Thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/522727a1dffb2dc93da5a0363224031d1152ab08

commit 522727a1dffb2dc93da5a0363224031d1152ab08
Author: Stephen Martinis <martiniss@chromium.org>
Date: Tue Sep 25 22:51:46 2018

Enable patch de-application for mac_optional_gpu_tests_rel

Led-Recipes-Tester-Builder: luci.chromium.try:mac_optional_gpu_tests_rel
Bug:  883308 
Change-Id: Ida0149c578683958cc9a53848a6d2491640b0d6e
Reviewed-on: https://chromium-review.googlesource.com/1232138
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Ben Pastene <bpastene@chromium.org>

[modify] https://crrev.com/522727a1dffb2dc93da5a0363224031d1152ab08/scripts/slave/recipe_modules/chromium_tests/trybots.py

I haven't seen this logic get triggered on the bot right now. The only build which looks suspicious in the past day is https://ci.chromium.org/p/chromium/builders/luci.chromium.try/mac_optional_gpu_tests_rel/9847. That looks like there was a network issue; the gl tests didn't flake.

I'm tentatively going to close this; if anything breaks, feel free to re-open and I'll take a look.

Thank you Stephen and Kai for collaborating to fix this! Excellent work!