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