Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2068793002:
Allow any Chromium trybot to run webkit_tests. (Closed)
|
Created:
4 years, 6 months ago by bungeman-chromium Modified:
4 years, 4 months ago CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/tools/build@master Target Ref:
refs/heads/master Project:
build Visibility:
Public. |
DescriptionAllow any Chromium trybot to run webkit_tests.
Check for the property 'force_blink_tests' and, if specified, use its
value to override the default setting for running the webkit_tests step.
The command 'git cl try -p force_blink_tests=True' may be used to
specify this property. The immediate use case is to run webkit_tests on
Windows 8 and 10 trybots.
BUG=chromium:620356
Patch Set 1 #
Messages
Total messages: 18 (7 generated)
The CQ bit was checked by bungeman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2068793002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-infra-committers". Note that this has nothing to do with OWNERS files.
bungeman@chromium.org changed reviewers: + dpranke@chromium.org
This is a proposed solution to a feature request I have. Specifically, with https://crrev.com/2065833002 glyph rendering will be different for at least some layout tests between Windows 7 and later versions. It is currently difficult to see how changes will impact the layout tests ahead of time since currently it appears all of the Windows trybots which run layout tests are Windows 7. With a change like this it would be possible to specify that the webkit_tests step should be run on the existing Windows 8 and 10 bots.
dpranke@chromium.org changed reviewers: + phajdan.jr@chromium.org
normally the way we decide what tests to run is by changing the matching entries in the //testing/buildbot/*.json files. However, you can't currently do that for webkit_tests, so in the absence of fixing that, this seems like the only viable solution. I like the idea of being able to specify a given test suite on a given trybot via a mechanism like this, but the way you've done this particular thing isn't very generic. As long as you're open to changing the mechanism if/when we have something better, this lgtm. Adding phadjan.jr@ as well in case he has thoughts / preferences / etc.
The CQ bit was checked by bungeman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2068793002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/06/14 21:45:37, Dirk Pranke wrote: > normally the way we decide what tests to run is by changing the matching entries > in the //testing/buildbot/*.json files. However, you can't currently do that for > webkit_tests, so in the absence of fixing that, this seems like the only viable > solution. > > I like the idea of being able to specify a given test suite on a given trybot > via a mechanism like this, but the way you've done this particular thing isn't > very generic. > > As long as you're open to changing the mechanism if/when we have something > better, this lgtm. > > Adding phadjan.jr@ as well in case he has thoughts / preferences / etc. It does seem that one could have a list of all of the names of all of the available tests and provide a property with a value which expresses which tests should be run or excluded (this could be very simple or very complicated). I was originally just going to open an issue, but I got far enough along to write this. I would have no problem with the mechanism changing, I just have a rather bad itch at the moment.
Why not modify CHROMIUM_BLINK_TESTS_BUILDERS instead? In general, I don't think ad-hoc properties like this are going to be supported. CQ is not going to accept a job using this and will run it again with default settings. I strongly suggest you come up with specific requirements, and start a thread on infra-dev or internal ML. Then we can find the best solution.
On 2016/06/15 14:36:27, Paweł Hajdan Jr. wrote: > Why not modify CHROMIUM_BLINK_TESTS_BUILDERS instead? That would affect every try job. I think bungeman@ wants something that is optional but can be triggered w/ git-cl try. > In general, I don't think ad-hoc properties like this are going to be supported. > CQ is not going to accept a job using this and will run it again with default > settings. I don't think he wants this to apply to tryjobs, but I agree that if he did this wouldn't be the way to do it.
On 2016/06/15 14:36:27, Paweł Hajdan Jr. wrote: > Why not modify CHROMIUM_BLINK_TESTS_BUILDERS instead? Because then those bots would be running webkit_tests on all runs (which isn't a bad idea) but means those bots would suddenly become more expensive to run. If this is acceptable, I would have no issue with that. > > In general, I don't think ad-hoc properties like this are going to be supported. > CQ is not going to accept a job using this and will run it again with default > settings. The issue I'm trying to work around is that BlinkTests *is* currently ad-hoc. I don't like adding ad-hoc properties either, but I would like to run webkit_tests on a Windows 8 and 10 trybot on a change that needs to land within the week, and I'm going to need to do so on an ongoing basis in the future once it lands. > > I strongly suggest you come up with specific requirements, and start a thread on > infra-dev or internal ML. Then we can find the best solution. I'm not sure how much more specific I can get. I want to run the webkit_tests step on any trybot. The 'best' solution is to stop making the trybots different from the waterfall bots so that every waterfall bot has a trybot which builds exactly the same and runs the same tests. An intermediate solution would be to make BlinkTests a bit less special so that a chromium/src side change can toggle the tests. The hacky sort term solution is the one here in Patch Set 1, or maybe adding to CHROMIUM_BLINK_TESTS_BUILDERS if that is acceptable. I'll open an issue.
On 2016/06/15 15:28:28, bungeman-skia wrote: > The issue I'm trying to work around is that BlinkTests *is* currently ad-hoc. I > don't like adding ad-hoc properties either, but I would like to run webkit_tests > on a Windows 8 and 10 trybot on a change that needs to land within the week, and > I'm going to need to do so on an ongoing basis in the future once it lands. We don't actually support Win8 in the webkit_tests anymore, so you don't need to worry about that one.
Description was changed from ========== Allow any Chromium trybot to run webkit_tests. Check for the property 'force_blink_tests' and, if specified, use its value to override the default setting for running the webkit_tests step. The command 'git cl try -p force_blink_tests=True' may be used to specify this property. The immediate use case is to run webkit_tests on Windows 8 and 10 trybots. ========== to ========== Allow any Chromium trybot to run webkit_tests. Check for the property 'force_blink_tests' and, if specified, use its value to override the default setting for running the webkit_tests step. The command 'git cl try -p force_blink_tests=True' may be used to specify this property. The immediate use case is to run webkit_tests on Windows 8 and 10 trybots. BUG=chromium:620356 ========== |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
