rebaseline-cl added unnecessary android-specific baseline for test that is skipped. |
||
Issue descriptionIn http://crrev.com/2419473003, the test imported/wpt/custom-elements/htmlconstructor/newtarget.html was updated, and this resulted in needing to make a new baseline; this baseline was not platform-specific, but still a copy of the old baseline was moved over to the android baseline directory. My initial thought is that the copying was done by `webkit-patch copy-existing-baselines-internal` and maybe there should be logic that prevents copying baselines to platform-specific directories for platforms where the test is skipped.
,
Oct 26 2016
After checking, I realized that copy-existing-baselines-internal already basically has the behavior that I want: https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py?l=195 (added in http://crrev.com/23135008). Haven't yet verified whether expectations.get_expectations(test_name) returns SKIP if the test is skipped because of not being in SmokeTests.
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f437383bbfd2cd52b8cacf0adff4e4fb17293127 commit f437383bbfd2cd52b8cacf0adff4e4fb17293127 Author: qyearsley <qyearsley@chromium.org> Date: Thu Oct 27 17:25:20 2016 Refactoring: Move _port_skips_test to Port class. As part of rebaselining, when copying existing baselines, we don't want to make new platform-specific baseline files for platforms where tests are skipped. In particular, we don't want to make new baselines for android for non-smoke-tests now since non-smoke-tests are skipped on android. There's already logic in the copy-existing-baselines to do this; it uses the TestExpectations.get_expectations method, which, as far as I understand, returns expectations based on test expectations files, but not based on whether the port only runs smoke tests. Meanwhile, elsewhere in rebaseline.py, there's already logic to check whether a test is skipped on a particular port including because it's not a smoke test. The purpose of this CL is to extract that method out so it could be re-used. An alternative to doing this would be to change TestExpectations.get_expectations to also include "SKIP" if port.default_smoke_test_only() and the test is in the smoke tests file. (Either way, there should be a centralized way of checking whether a test runs on a port, and this should be used in both places in rebaseline.py.) BUG= 655196 Review-Url: https://codereview.chromium.org/2450383002 Cr-Commit-Position: refs/heads/master@{#428067} [modify] https://crrev.com/f437383bbfd2cd52b8cacf0adff4e4fb17293127/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py [modify] https://crrev.com/f437383bbfd2cd52b8cacf0adff4e4fb17293127/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
,
Oct 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2543789d3b0638e3afed6efaf0a9e8e244a3c42 commit f2543789d3b0638e3afed6efaf0a9e8e244a3c42 Author: qyearsley <qyearsley@chromium.org> Date: Fri Oct 28 23:22:59 2016 Use Port.skips_test in copy-existing-baselines command. After this CL, I expect that when rebaselining (with rebaseline-cl or other rebaseline commands), new baselines shouldn't be added for non-smoke-tests on android. BUG= 655196 Review-Url: https://codereview.chromium.org/2455233002 Cr-Commit-Position: refs/heads/master@{#428540} [modify] https://crrev.com/f2543789d3b0638e3afed6efaf0a9e8e244a3c42/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py [modify] https://crrev.com/f2543789d3b0638e3afed6efaf0a9e8e244a3c42/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
,
Nov 1 2016
I think that this is considered fixed now; just tried it out and no extra baselines were added for android (test CL: https://codereview.chromium.org/2463043003). |
||
►
Sign in to add a comment |
||
Comment 1 by qyears...@chromium.org
, Oct 24 2016