OLD | NEW |
1 Blink layout tests | 1 Blink layout tests |
2 ================== | 2 ================== |
3 | 3 |
4 How to land Skia changes that change Blink layout test results. | 4 How to land Skia changes that change Blink layout test results. |
5 | 5 |
6 Changes that affect a small number of layout test results | 6 Changes that affect a small number of layout test results |
7 --------------------------------------------------------- | 7 --------------------------------------------------------- |
8 Changes affecting fewer than ~20 layout tests can be rebaselined without | 8 Changes affecting fewer than ~20 layout tests can be rebaselined without |
9 special coordination with the Blink gardener using these steps: | 9 special coordination with the Blink gardener using these steps: |
10 | 10 |
11 1. Prepare your Skia change, taking note of which layout tests will turn red | 11 1. Prepare your Skia change, taking note of which layout tests will turn red |
12 \(see http://www.chromium.org/developers/testing/webkit-layout-tests for more | 12 \(see http://www.chromium.org/developers/testing/webkit-layout-tests for more |
13 detail on running the Blink layout tests\). | 13 detail on running the Blink layout tests\). |
14 2. Check in your code to the Skia repo. | 14 2. Check in your code to the Skia repo. |
15 3. Ahead of the Skia auto roll including your change, manually push a change to
the | 15 3. Ahead of the Skia auto roll including your change, manually push a change to
the |
16 Blink LayoutTests/TestExpectations [file](http://src.chromium.org/viewvc/blin
k/trunk/LayoutTests/TestExpectations), | 16 Blink LayoutTests/TestExpectations [file](https://chromium.googlesource.com/c
hromium/src/+/master/third_party/WebKit/LayoutTests/TestExpectations), |
17 flagging tests expected to fail as a result of your change with \[ NeedsManua
lRebaseline \]. | 17 flagging tests expected to fail as a result of your change with \[ NeedsManua
lRebaseline \]. |
18 4. Wait for the Skia roll to land successfully. | 18 4. Wait for the Skia roll to land successfully. |
19 5. Check in another change to the Blink TestExpectations file changing the flags
to | 19 5. Check in another change to the Blink TestExpectations file changing the flags
to |
20 \[ NeedsRebaseline\], which will prompt the automatic rebaseline. | 20 \[ NeedsRebaseline\], which will prompt the automatic rebaseline. |
21 | 21 |
22 | 22 |
23 | 23 |
24 Changes that affect a large number of test results | 24 Changes that affect a large number of test results |
25 -------------------------------------------------- | 25 -------------------------------------------------- |
26 Where a 'large number' or 'many' means more than about 20. | 26 Where a 'large number' or 'many' means more than about 20. |
27 Follow the instructions below: | 27 Follow the instructions below: |
28 | 28 |
29 In the following the term 'code suppression' means a build flag \(a\.k\.a\. defi
ne\). | 29 In the following the term 'code suppression' means a build flag \(a\.k\.a\. defi
ne\). |
30 Such code suppressions should be given a name with the form SK\_IGNORE\_xxx\_FIX
. | 30 Such code suppressions should be given a name with the form SK\_IGNORE\_xxx\_FIX
. |
31 | 31 |
32 There are dependency revisions which must be updated in this process. Updating | 32 Updating the version of Skia in Chromium is called a 'roll'. |
33 a dependency revision is called a 'roll'. There are two different rolls which | 33 The Auto Roll Bot performs this roll multiple times per day, and can also be don
e manually. |
34 concern this process, each of which happens via an Auto Roll Bot multiple | 34 See https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search f
or skia\-deps\-roller. |
35 times per day, and can also be done manually: | |
36 | |
37 * Skia roll into Chromium. See | |
38 https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search for | |
39 skia\-deps\-roller. | |
40 * Blink roll into Chromium. See | |
41 https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search for | |
42 blink\-deps\-roller. | |
43 | 35 |
44 ### Setup | 36 ### Setup |
45 #### Code suppression does not yet exist \- Direct method | 37 #### Code suppression does not yet exist \- Direct method |
46 1. Make a change in Skia which will change many Blink layout tests. | 38 1. Make a change in Skia which will change many Blink layout tests. |
47 2. Put the change behind a code suppression. | 39 2. Put the change behind a code suppression. |
48 3. Check in the change to the Skia repository. | 40 3. Check in the change to the Skia repository. |
49 4. Manually roll Skia or append the autoroll with the code suppression to | 41 4. Manually roll Skia or append the autoroll with the code suppression to |
50 Chromium's 'skia/skia\_common\.gypi' | 42 Chromium's 'skia/chromium\_skia\_defines\.gypi' |
51 5. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp'. | |
52 6. Wait for Blink roll into Chromium. | |
53 7. Remove code suppression from Chromium's 'skia/skia\_common\.gypi'. | |
54 | 43 |
55 #### Code suppression does not yet exist \- Alternate method | 44 #### Code suppression does not yet exist \- Alternate method |
56 1. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp' before maki
ng code | 45 1. Add code suppression to Chromium's 'skia/chromium\_skia\_defines\.gypi' befor
e making code |
57 changes in Skia. | 46 changes in Skia. |
58 2. Make a change in Skia which will change many Blink layout tests. | 47 2. Make a change in Skia which will change many Blink layout tests. |
59 3. Put the change behind a code suppression. | 48 3. Put the change behind a code suppression. |
60 4. Wait for Blink roll into Chromium. | 49 4. Check in the change to the Skia repository. |
61 5. Check in the change to the Skia repository. | 50 5. Wait for Skia roll into Chromium. |
62 6. Wait for Skia roll into Chromium. | |
63 | 51 |
64 #### Code suppression exists in header | 52 #### Code suppression exists in header |
65 1. Remove code suppression from header file in Chromium and add code suppression
to | 53 1. Remove code suppression from header file in Chromium and add code suppression
to |
66 Chromium's 'skia/skia\_common\.gypi'. | 54 Chromium's 'skia/chromium\_skia\_defines\.gypi'. |
67 The code suppression cannot be in a header file and a defined in a gyp file a
t the | 55 The code suppression cannot be in a header file and a defined in a gyp file a
t the |
68 same time or a multiple definition warning will be treated as an error and br
eak | 56 same time or a multiple definition warning will be treated as an error and br
eak |
69 the Chromium build. | 57 the Chromium build. |
70 2. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp'. | |
71 3. Wait for Blink roll into Chromium. | |
72 4. Remove code suppression from Chromium's 'skia/skia\_common\.gypi'. | |
73 | 58 |
74 ### Rebaseline | 59 ### Rebaseline |
75 1. Choose a time when the Blink tree is likely to be quiet. Avoid PST afternoons
in | 60 1. Choose a time when the Blink tree is likely to be quiet. Avoid PST afternoons
in |
76 particular. The bigger the change, the more important this is. Regardless, | 61 particular. The bigger the change, the more important this is. Regardless, |
77 determine who the Blink gardener is and notify them. You will be making the | 62 determine who the Blink gardener is and notify them. You will be making the |
78 Chromium\.WebKit tree very red for an extended period, and the gardener needs
to | 63 Chromium\.WebKit tree very red for an extended period, and the gardener needs
to |
79 know that they are not expected to fix it. | 64 know that they are not expected to fix it. |
80 2. Create a CL removing the code suppression from Blink's | 65 2. Create a CL removing the code suppression from Chromium's |
81 public/blink_skia_config.gyp while simultaneously adding [ NeedsRebaseline ] | 66 skia/chromium\_skia\_defines\.gypi while simultaneously adding [ NeedsRebasel
ine ] |
82 lines to Blink's LayoutTests/TestExpectations [file](http://src.chromium.org/
viewvc/blink/trunk/LayoutTests/TestExpectations). | 67 lines to Blink's LayoutTests/TestExpectations [file](https://chromium.googles
ource.com/chromium/src/+/master/third_party/WebKit/LayoutTests/TestExpectations)
. |
83 Then the auto rebaseline bot will take care of the work of actually checking
in the | 68 Then the auto rebaseline bot will take care of the work of actually checking
in the |
84 new images. This is generally acceptable for up to 600 or so rebaselined imag
es. | 69 new images. This is generally acceptable for up to 600 or so rebaselined imag
es. |
85 Above that you might still use [ NeedsRebaseline ], but it's best to coordina
te with | 70 Above that you might still use [ NeedsRebaseline ], but it's best to coordina
te with |
86 the gardener/sheriff. This should go through the CQ cleanly. | 71 the gardener/sheriff. This should go through the CQ cleanly. |
87 3. Be careful with tests that are already failing or flakey. These may or may no
t need | 72 3. Be careful with tests that are already failing or flakey. These may or may no
t need |
88 to be rebaselined and flakey tests should not be removed from TestExpectation
s | 73 to be rebaselined and flakey tests should not be removed from TestExpectation
s |
89 regardless. In such cases revert the TestExpectations changes before committi
ng. | 74 regardless. In such cases revert the TestExpectations changes before committi
ng. |
90 4. If you are not the one handling the cleanup step, please open a Skia Issue of
the | 75 4. If you are not the one handling the cleanup step, please open a Skia Issue of
the |
91 form | 76 form |
92 Title: "Remove code suppression SK\_IGNORE\_xxx\_FIX\." | 77 Title: "Remove code suppression SK\_IGNORE\_xxx\_FIX\." |
93 Comment: "Code suppression SK\_IGNORE\_xxx\_FIX rebaselined with Blink revisi
on | 78 Comment: "Code suppression SK\_IGNORE\_xxx\_FIX rebaselined with Blink revisi
on |
94 123456\." and assign it to the individual responsible for the cleanup step. | 79 123456\." and assign it to the individual responsible for the cleanup step. |
95 | 80 |
96 ### Cleanup | 81 ### Cleanup |
97 1. Wait for Blink roll into Chromium, so that Chromium is using the new Skia cod
e | 82 1. Remove the now unused old code from Skia and any defines which were introduce
d |
98 and new Blink baselines. | |
99 2. Remove the now unused old code from Skia and any defines which were introduce
d | |
100 to suppress the new code. | 83 to suppress the new code. |
101 3. Check in the cleanup change to the Skia repository. | 84 2. Check in the cleanup change to the Skia repository. |
102 4. Wait for Skia roll into Chromium. | 85 3. Wait for Skia roll into Chromium. |
103 | 86 |
OLD | NEW |