Simplify the baseline-resetting commands of run-webkit-tests. |
||||||
Issue descriptionThere are a few baseline-resetting options for run-webkit-tests: --reset-results --new-test-results --new-baseline From Dirk in http://crrev.com/2456073002: > You can run run-webkit-tests in a way such that it ignores what the existing baselines > are and just generates new baselines. In that situation, there are three different options > for controlling where new baselines are put: --new-test-results which does what you say, > --new-baseline which writes to baseline_platform_dir(), and --reset-results, which > overwrites the existing baseline used on the port, whereever it is found. > The theory is that you might know whether the test results should produce one kind of > result or another and can then put the new baselines in the right place. > All of this long predates rebaseline-o-matic and rebaseline-cl. It's unclear to me which > of these options are still being used. --new-baseline is probably the least commonly > used, and it might be that only --reset-results is really useful. The reason that's useful > is because lots of tests only have one generic baseline, and you can know you can safely > just update it in your initial patch and not have to gather any platform-specific versions. By removing one or two of these flags, we could probably simplify some code in webkitpy.
,
Jun 3 2017
I would like to continue the discussion brought up by https://chromium-review.googlesource.com/c/522916/ (which adds --new-baseline-copy). After reading qyearsley@ and wkorman@'s comments and examining my use cases for --enable-slimming-paint-v2 flag-specific baselines, I found I don't actually need --new-baseline, but need --new-flag-specific-baseline. Consider if we use --new-baseline on mac, we don't want the new baselines to be created under directories like LayoutTests/flag-specific/enable-slimming-paint-v2/platform/mac-10.11/... We might want the new baselines to be under LayoutTests/flag-specific/enable-slimming-paint-v2/... instead. (With my current workflow on Linux, the new baselines are under LayoutTests/flag-specific/enable-slimming-paint-v2/platform/linux/...) Without the flag-specific use case, I wonder if any one would use --new-baseline. --reset-results can create new baselines if the baselines are missing, so we don't actually need --new-test-results. So --reset-results might be the only useful option in the three. For the flag-specific usage, I would like to add the following new options: --new-flag-specific-baseline (create new baselines with actual results) --new-flag-specific-baseline-copy (create new baselines by copying from the current baselines) They can be implemented based on the current implementation for --new-baseline. WDYT?
,
Jun 3 2017
+skobes who created the flag-specific feature.
,
Jun 3 2017
,
Jun 4 2017
Now I think we don't need --new-flag-specific-baseline-copy for code review. I added a section in https://chromium-review.googlesource.com/c/522916/4/docs/testing/layout_tests.md to describe the procedure taking advantage of the try bot to ease code review.
,
Jun 5 2017
skobes@ what do you think about the workflow for rebaselining flag-specific expectations described in https://chromium-review.googlesource.com/c/522916/4/docs/testing/layout_tests.md? Would you setup a try bot for RLS?
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cacba4879996384350b615f7578ff832230c0e2e commit cacba4879996384350b615f7578ff832230c0e2e Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Mon Jun 05 20:36:10 2017 Deprecate --new-baseline and --new-test-results Use --reset-results instead of --new-test-results. Use "webkit-patch rebaseline-cl" or "--reset-results --add-platform-exceptions" instead of --new-baseline. Still keep "--add-platform-exceptions" to create platform-version-specific baselines. May remove it in the future if no one wants it. BUG= 660231 Change-Id: I9671c69a806e5cba1ccd838944e757a364a947a2 Reviewed-on: https://chromium-review.googlesource.com/523386 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Reviewed-by: Walter Korman <wkorman@chromium.org> Cr-Commit-Position: refs/heads/master@{#477075} [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/docs/testing/layout_tests.md [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_input.py [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/printing.py [modify] https://crrev.com/cacba4879996384350b615f7578ff832230c0e2e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py
,
Jun 5 2017
This will be useful for RLS. So far we haven't added a significant number of flag-specific baselines but we do expect to have some (though perhaps fewer than I initially thought, after http://crrev.com/c/524267). We've had some discussion of setting up a bot, but for now we're updating flag expectations manually. The proposed review workflow sounds fine. Another option would be to upload two patch sets, the first having the generic baselines copied into the flag-specific directory, and the second having the correct flag-specific baselines. Then the reviewer can see the diff between the patch sets.
,
Jun 5 2017
I can add --new-flag-specific-baseline-copy option that I proposed in #2. It copies the baselines only if the new flag-specific baselines will be different, to avoid unnecessary rebaselines. The code had been ready but removed from the final patch of https://chromium-review.googlesource.com/c/522916/.
,
Jun 5 2017
I like that... actually if we had --new-flag-specific-baseline-copy we wouldn't need --new-flag-specific-baseline, since --reset-results will update an existing flag-specific baseline, right?
,
Jun 5 2017
You can treat "run-webkit-tests --new-flag-specific-baseline" as a shortcut of "run-webkit-tests --new-flag-specific-baseline-copy" and "run-webkit-tests --reset-results" :)
,
Jul 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7dbd719a57553e94a2d1dfc9414a8d013749fc54 commit 7dbd719a57553e94a2d1dfc9414a8d013749fc54 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue Jul 04 01:39:36 2017 Remove the current non-generic baseline before rebaselining it This avoids unnecessary non-generic baselines which are the same as the fallback baseline. This is similar to "webkit-patch optimize-baselines" but avoids extra baselines at the first place. For example, this is useful for optimizing flag-specific baselines in the following process: 1. We created flag-specific baseline for a test when it produced result different from the existing expectations. 2. Then some code change makes the test produce flag-specific result the same as the original existing expectations before step 1. 3. We use run-webkit-tests --new-flag-specific-baseline <test>. Previously at step 3, we would overwrite the flag-specific baseline with the new actual result. Now we just remove the flag-specific baseline. BUG= 660231 Change-Id: I713c9d729f60f88ff29e46f3b995433dcdf36ba1 Reviewed-on: https://chromium-review.googlesource.com/558380 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#484040} [modify] https://crrev.com/7dbd719a57553e94a2d1dfc9414a8d013749fc54/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py [modify] https://crrev.com/7dbd719a57553e94a2d1dfc9414a8d013749fc54/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
,
Oct 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d063968e93165fd681b83146cb9aeca885886446 commit d063968e93165fd681b83146cb9aeca885886446 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Mon Oct 16 16:47:44 2017 run-webkit-tests --copy-baselines --copy-baselines: If the actual result is different from the current baseline, copy the current baseline into the *most-specific-platform* directory, or the flag-specific generic-platform directory if --additional-driver-flag is specified. See --reset-results. --reset-results: Reset expectations to the generated results in their existing location. If --copy-baselines is specified, the copied baselines will be reset. --new-flag-specific-baseline: Deprecated. Replaced by --copy-baselines --reset-results --add-platform-exceptions: Deprecated. The separation of --new-flag-specific-baseline into --copy-baselines and --reset-results (existing) is to ease code review of flag-specific rebaseline CLs. The developer can use --copy-baselines to generate the first patch set, then use --reset-results to generate the second patch set. The reviewer compares the first and the second patch sets to see the difference of the new baselines. Bug: 660231 Change-Id: Ia91a92ddb9e260b20566e49705f551df13ccacc3 Reviewed-on: https://chromium-review.googlesource.com/713760 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Steve Kobes <skobes@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Cr-Commit-Position: refs/heads/master@{#509077} [modify] https://crrev.com/d063968e93165fd681b83146cb9aeca885886446/docs/testing/layout_tests.md [modify] https://crrev.com/d063968e93165fd681b83146cb9aeca885886446/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py [modify] https://crrev.com/d063968e93165fd681b83146cb9aeca885886446/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py [modify] https://crrev.com/d063968e93165fd681b83146cb9aeca885886446/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
,
Oct 16 2017
,
Oct 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5ca1d27a4a67ec9121c5382c274401f5039d44bd commit 5ca1d27a4a67ec9121c5382c274401f5039d44bd Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Fri Oct 27 01:44:56 2017 [run-webkit-tests] Let --reset-results reset flag-specific baselines If --additional-driver-flag is specified, it was weird to still let --reset-results reset the existing baselines. There seems no use case for that. Now let --reset-results reset flag-specific baselines if --additional- driver-flag is specified. Bug: 660231 Change-Id: I6ee5a5f563a7f972fcece763256d7f99f3e97cbe Reviewed-on: https://chromium-review.googlesource.com/740841 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#512061} [modify] https://crrev.com/5ca1d27a4a67ec9121c5382c274401f5039d44bd/docs/testing/layout_tests.md [modify] https://crrev.com/5ca1d27a4a67ec9121c5382c274401f5039d44bd/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py [modify] https://crrev.com/5ca1d27a4a67ec9121c5382c274401f5039d44bd/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py [modify] https://crrev.com/5ca1d27a4a67ec9121c5382c274401f5039d44bd/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by qyears...@chromium.org
, Apr 6 2017