Joe Gregorio | 02f7202 | 2021-03-27 10:12:45 -0400 | [diff] [blame] | 1 | |
| 2 | --- |
| 3 | title: "Blink layout tests" |
| 4 | linkTitle: "Blink layout tests" |
| 5 | |
| 6 | --- |
| 7 | |
| 8 | |
| 9 | How to land Skia changes that change Blink layout test results. |
| 10 | |
| 11 | Changes that affect a small number of layout test results |
| 12 | --------------------------------------------------------- |
| 13 | Changes affecting fewer than ~20 layout tests can be rebaselined without |
| 14 | special coordination with the Blink gardener using these steps: |
| 15 | |
| 16 | 1. Prepare your Skia change, taking note of which layout tests will turn red |
| 17 | \(see http://www.chromium.org/developers/testing/webkit-layout-tests for more |
| 18 | detail on running the Blink layout tests\). |
| 19 | 2. Check in your code to the Skia repo. |
| 20 | 3. Ahead of the Skia auto roll including your change, manually push a change to the |
Eric Boren | 04fe267 | 2021-09-27 10:15:39 -0400 | [diff] [blame] | 21 | Blink LayoutTests/TestExpectations [file](https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/web_tests/TestExpectations), flagging tests expected to fail as a result of your change as follows: |
Joe Gregorio | 02f7202 | 2021-03-27 10:12:45 -0400 | [diff] [blame] | 22 | foo/bar/test-name.html [ Failure Pass ] # Needs rebaseline |
| 23 | |
| 24 | 4. Wait for the Skia roll to land successfully. |
| 25 | 5. Check in another change to the Blink TestExpectations file removing all the |
| 26 | skipped test expectations you add earlier, an run `git cl rebaseline` which will prompt the automatic rebaseline. |
| 27 | |
| 28 | |
| 29 | |
| 30 | Changes that affect a large number of test results |
| 31 | -------------------------------------------------- |
| 32 | Where a 'large number' or 'many' means more than about 20. |
| 33 | Follow the instructions below: |
| 34 | |
| 35 | In the following the term 'code suppression' means a build flag \(a\.k\.a\. define\). |
| 36 | Such code suppressions should be given a name with the form SK\_IGNORE\_xxx\_FIX. |
| 37 | |
| 38 | Updating the version of Skia in Chromium is called a 'roll'. |
| 39 | The Auto Roll Bot performs this roll multiple times per day, and can also be done manually. |
Eric Boren | 04fe267 | 2021-09-27 10:15:39 -0400 | [diff] [blame] | 40 | See https://chromium.googlesource.com/chromium/src/+log/main/DEPS and search for skia\-deps\-roller. |
Joe Gregorio | 02f7202 | 2021-03-27 10:12:45 -0400 | [diff] [blame] | 41 | |
| 42 | ### Setup |
| 43 | #### Code suppression does not yet exist \- Direct method |
| 44 | 1. Make a change in Skia which will change many Blink layout tests. |
| 45 | 2. Put the change behind a code suppression. |
| 46 | 3. Check in the change to the Skia repository. |
| 47 | 4. Manually roll Skia or append the autoroll with the code suppression to |
| 48 | Chromium's 'skia/chromium\_skia\_defines\.gypi' |
| 49 | |
| 50 | #### Code suppression does not yet exist \- Alternate method |
| 51 | 1. Add code suppression to Chromium's 'skia/chromium\_skia\_defines\.gypi' before making code |
| 52 | changes in Skia. |
| 53 | 2. Make a change in Skia which will change many Blink layout tests. |
| 54 | 3. Put the change behind a code suppression. |
| 55 | 4. Check in the change to the Skia repository. |
| 56 | 5. Wait for Skia roll into Chromium. |
| 57 | |
| 58 | #### Code suppression exists in header |
| 59 | 1. Remove code suppression from header file in Chromium and add code suppression to |
| 60 | Chromium's 'skia/chromium\_skia\_defines\.gypi'. |
| 61 | The code suppression cannot be in a header file and a defined in a gyp file at the |
| 62 | same time or a multiple definition warning will be treated as an error and break |
| 63 | the Chromium build. |
| 64 | |
| 65 | ### Rebaseline |
| 66 | 1. Choose a time when the Blink tree is likely to be quiet. Avoid PST afternoons in |
| 67 | particular. The bigger the change, the more important this is. Regardless, |
| 68 | determine who the Blink gardener is and notify them. You will be making the |
| 69 | Chromium\.WebKit tree very red for an extended period, and the gardener needs to |
| 70 | know that they are not expected to fix it. |
| 71 | 2. Create a CL removing the code suppression from Chromium's |
| 72 | skia/chromium\_skia\_defines\.gypi while simultaneously adding [ NeedsRebaseline ] |
Eric Boren | 04fe267 | 2021-09-27 10:15:39 -0400 | [diff] [blame] | 73 | lines to Blink's LayoutTests/TestExpectations [file](https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/web_tests/TestExpectations). |
Joe Gregorio | 02f7202 | 2021-03-27 10:12:45 -0400 | [diff] [blame] | 74 | Then the auto rebaseline bot will take care of the work of actually checking in the |
| 75 | new images. This is generally acceptable for up to 600 or so rebaselined images. |
| 76 | Above that you might still use [ NeedsRebaseline ], but it's best to coordinate with |
| 77 | the gardener. This should go through the CQ cleanly. |
| 78 | 3. Be careful with tests that are already failing or flakey. These may or may not need |
| 79 | to be rebaselined and flakey tests should not be removed from TestExpectations |
| 80 | regardless. In such cases revert the TestExpectations changes before committing. |
| 81 | 4. If you are not the one handling the cleanup step, please open a Skia Issue of the |
| 82 | form |
| 83 | Title: "Remove code suppression SK\_IGNORE\_xxx\_FIX\." |
| 84 | Comment: "Code suppression SK\_IGNORE\_xxx\_FIX rebaselined with Blink revision |
| 85 | 123456\." and assign it to the individual responsible for the cleanup step. |
| 86 | |
| 87 | ### Cleanup |
| 88 | 1. Remove the now unused old code from Skia and any defines which were introduced |
| 89 | to suppress the new code. |
| 90 | 2. Check in the cleanup change to the Skia repository. |
| 91 | 3. Wait for Skia roll into Chromium. |
| 92 | |
| 93 | |