New issue
Advanced search Search tips

Issue 605496 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocked on:
issue 711515



Sign in to add a comment

Move run-webkit-tests to src-side runtests.py, or stop using runtests.py.

Project Member Reported by tansell@chromium.org, Apr 21 2016

Issue description

When chatting to Pawel about https://codereview.chromium.org/1911673003, he said;
--------------------------
I recommend a different approach
My recommendation would be: why don't you switch to src-side runtest.py, or even ditch runtest.py entirely?
You're moving in a very slow territory though.
build-side changes are hard to test and risky
whereas with src-side, you could just test on trybots
as the person who's #2 in number of commits in build repo, I think I have the experience to make that recommendation
--------------------------

This sounds like something we should investigate and understand if we are making the right choice here.

Pawel, can you elaborate a little more why we would want to spend the effort here?

 
There is some information at https://bugs.chromium.org/p/chromium/issues/detail?id=506498
Status: Available (was: Untriaged)
I think it is a good idea to move as much src-side as we can.
Is this bug about the run_webkit_tests.py in chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/? Or is there another run_webkit_tests?
This is about the "webkit_tests" step in the build process. See this example run https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/215344
As a result of work done while debugging bug 506498, I figured out how hard this would be, and the answer is "not very".

If you were to revert https://codereview.chromium.org/1936943002 (i.e., reland the underlying changes) and then make run-webkit-tests smart enough to launch an xvfb instance on linux you could call it directly.

Or you could just invoke run-webkit-tests via //testing/xvfb.py.

Either way, you at least get to delete the layout_test_wrapper.py which is 271 lines of dead code, bugs, and confusion, and you can cut out 1-2 layers of python indirection.

Comment 6 Deleted

Comment 7 Deleted

Components: -Infra
Cc: -dcampb@google.com
Labels: -Type-Bug -Pri-3 Pri-2 Type-Task
Summary: Move run-webkit-tests to src-side runtests.py, or stop using runtests.py. (was: Should run_webkit_tests should move to src side runtests.py or even remove it)
In recipe_modules/chromium_tests/steps.py, runtest is used here:
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/steps.py?l=2214

This is related to  bug 695700  (stop using layout_test_wrapper.py).

This issue doesn't apply to the layout test using swarming, which currently use src/testing/scripts/run_isolated_script_test.py and not runtest.py.
Blockedon: 711515
qyearsley@,  issue 711515  is now fixed, does that unblock this issue?

(Doing triage of Blink>Infra P2 issues with no activity in 60+ days.)
Cc: qyears...@chromium.org
As far as I know, this issue is unblocked but still not quite finished.

Now, layout tests are run on swarming on all platforms except Android (thanks to Tim and Dirk). The BlinkTest class in recipe_modules/chromium_tests/steps.py still hasn't been removed because it's still used for Android, and runtest.py is still used in that case.

Things that will solve this issue:
 - Change android to not use runtest.py by simplifying that code in BlinkTest. Maybe this would just work?
 - Run layout tests on swarming on Android.

Dirk, what's still required for layout tests running on swarming on Android?
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 24 2018

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

commit 7d41ee128093531c0b8fa07f0c6770be9b586e35
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Wed Jan 24 22:06:05 2018

chromium_tests BlinkTest: Invoke rwt without runtest.py.

This would change the way that run-webkit-tests is invoked
on Android so that runtest.py is not used. I don't remebmer
any reason why runtest.py might be necessary besides xvfb,
so maybe we can stop using it here.

Bug:  605496 
Change-Id: I3e29e659f23872e88ba4b51fa907be8bc3cc30c3
Reviewed-on: https://chromium-review.googlesource.com/846266
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>

[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipe_modules/chromium_tests/tests/steps/blink_test.expected/android.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Mac_fail.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/webkit_tests_interrupted.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Linux_64_fail.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Linux_64__dbg__pass.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipe_modules/chromium_tests/tests/steps/blink_test.expected/big.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Linux_64_pass.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Linux_64__dbg__fail.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Linux_64___future_fail.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/minimal_pass_continues.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/webkit_tests_unexpected_error.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipe_modules/chromium_tests/steps.py
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipe_modules/chromium_tests/tests/steps/blink_test.expected/unexpected_flakes.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/too_many_failures_for_retcode.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Linux_64___future_pass.json
[modify] https://crrev.com/7d41ee128093531c0b8fa07f0c6770be9b586e35/scripts/slave/recipes/blink_downstream.expected/full_client_v8_fyi_V8_Blink_Mac_pass.json

Cc: -phajdan.jr@chromium.org -qyears...@chromium.org
Owner: qyears...@chromium.org
Status: Fixed (was: Available)
Now run-webkit-tests no longer uses runtest.py on the bots.

Sign in to add a comment