Skip to content

Commit e77093b

Browse files
"Fix failing checks" wiki page for new contributors (flutter#154629)
Resolves flutter#154628 <br> As a contributor, I would expect resources related to testing to be located in the [/docs/contributing/testing](https://github.com/flutter/flutter/tree/master/docs/contributing/testing) directory. This PR adds a page to that directory geared toward helping new contributors. <br> I've noticed that in `team-framework` triage meetings, comments along the lines of "could you take a look at the failing checks?" are a regular occurrence. I believe we could save quite a bit of effort by changing this: > [**Understanding a LUCI build failure**](https://github.com/flutter/flutter/blob/a5ca16ea94048103b110d2cbd41e95d770735fb5/docs/infra/Understanding-a-LUCI-build-failure.md) to this: > [**Flutter wiki � How to fix failing checks**](https://github.com/nate-thegrate/flutter/blob/fix-checks-wiki-page/docs/contributing/testing/Fix-failing-checks.md)
1 parent ed553d1 commit e77093b

File tree

2 files changed

+140
-0
lines changed

2 files changed

+140
-0
lines changed
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
![Some checks were not successful](https://github.com/user-attachments/assets/95fd56e9-4839-4944-b9ac-cc45404896a2)
2+
3+
# How to fix a PR's failing checks
4+
5+
<br>
6+
7+
### tree-status
8+
9+
![Tree is currently broken.](https://github.com/user-attachments/assets/b611d540-c4cb-47dc-a27f-bef8709f24ce)
10+
11+
Unlike other checks, **tree-status** isn't tied to the pull request:
12+
instead, it shows whether checks are passing in the main branch.\
13+
Failures can happen for a variety of reasons and should be addressed
14+
before anything else is merged in.
15+
16+
**What to do:** Once [review requirements](../Tree-hygiene.md#getting-a-code-review)
17+
are met and all other checks are passing, a reviewer will add the
18+
[**`autosubmit`**](../../infra/Landing-Changes-With-Autosubmit.md) label,
19+
and then a bot will merge the PR once **tree-status** succeeds.
20+
21+
<br>
22+
23+
### Google testing
24+
25+
![Google testing](https://github.com/user-attachments/assets/7d1f9a66-b84a-4223-b57d-77b44f205d1c)
26+
27+
A Google testing failure could be a flake ([see below](#flaking)), or it
28+
might be due to changes in the PR (See
29+
[Understanding Google Testing](../../infra/Understanding-Google-Testing.md)
30+
for more info).
31+
Google employees can view the test output and give feedback accordingly.
32+
33+
**What to do:** If 2 weeks have gone by and nobody's looked into it,
34+
feel free to [reach out on Discord](../Chat.md).
35+
36+
<br>
37+
38+
### ci.yaml validation
39+
40+
![ci.yaml validation](https://github.com/user-attachments/assets/545a55f8-5bde-460f-92dd-9d87788f9fe8)
41+
42+
In order for checks to run correctly, the [.ci.yaml](../../../.ci.yaml)
43+
file needs to stay in sync with the base branch.
44+
45+
**What to do:** This check failure can be fixed by applying the latest changes
46+
from master.\
47+
(The [Tree hygiene](../Tree-hygiene.md#using-git) page recommends updating
48+
via rebase, rather than a merge commit.)
49+
50+
![Update with rebase](https://github.com/user-attachments/assets/8bacd87f-410a-4a9c-8ad0-075dd05f3eff)
51+
52+
<br>
53+
54+
## A bug in the PR
55+
56+
Oftentimes, a change inadvertently breaks expected behavior.\
57+
When this happens, usually the best way to find out what's wrong is to
58+
[**view the test output**](#view-the-test-output).
59+
60+
If a **customer_testing** check is unsuccessful, it's a signal that something in the
61+
[Flutter customer test registry](https://github.com/flutter/tests/) has failed.
62+
This includes [package tests](../../ecosystem/testing/Understanding-Packages-tests.md)
63+
along with other tests from open-source Flutter projects.\
64+
If a pull request requires an update to those external tests, it qualifies as a
65+
[**breaking change**](../Tree-hygiene.md#handling-breaking-changes);
66+
please avoid those when possible.
67+
68+
If **Linux Analyze** fails, it's likely that one or more changes in the PR
69+
violated a [linter rule](https://dart.dev/lints/).\
70+
Consider reviewing the steps outlined in
71+
[setting up the framework dev environment](../../Setting-up-the-Framework-development-environment.md)
72+
so that most of these problems get caught in static analysis right away.
73+
74+
> [!NOTE]
75+
> All Dart code is run through static analysis:
76+
> this includes markdown code snippets in doc comments!
77+
>
78+
> See [Hixie's Natural Log](https://ln.hixie.ch/?start=1660174115) for more details.
79+
80+
<br>
81+
82+
### View the test output
83+
84+
Click on **Details** for the failing test, and then click
85+
**View more details on flutter-dashboard**.
86+
87+
![view more details](https://github.com/user-attachments/assets/df667176-205f-42b2-8997-885c50ab238d)
88+
89+
The full test output is linked at the bottom of the page.
90+
91+
![LUCI overview page](https://github.com/user-attachments/assets/9603c6ad-90ec-47e1-96e8-9e3430f2c1b8)
92+
93+
<br>
94+
95+
Often, there will be a message that resembles the one below:
96+
97+
```
98+
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
99+
The following TestFailure was thrown running a test:
100+
Expected: exactly one matching candidate
101+
Actual: _TextWidgetFinder:<Found 0 widgets with text
102+
"AsyncSnapshot<String>(ConnectionState.waiting, null, null, null)": []>
103+
Which: means none were found but one was expected
104+
105+
When the exception was thrown, this was the stack:
106+
#4 main.<anonymous closure>.<anonymous closure> (…/packages/flutter/test/widgets/async_test.dart:115:7)
107+
<asynchronous suspension>
108+
#5 testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
109+
<asynchronous suspension>
110+
#6 TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1032:5)
111+
<asynchronous suspension>
112+
<asynchronous suspension>
113+
(elided one frame from package:stack_trace)
114+
115+
This was caught by the test expectation on the following line:
116+
file:///b/s/w/ir/x/w/flutter/packages/flutter/test/widgets/async_test.dart line 115
117+
The test description was:
118+
gracefully handles transition from null future
119+
════════════════════════════════════════════════════════════════════════════════════════════════════
120+
```
121+
122+
From there, it's just a matter of finding the failing test,
123+
[running it locally](./Running-and-writing-tests.md),
124+
and figuring out how to fix it!
125+
126+
<br>
127+
128+
### Flaking
129+
130+
A check might "flake", or randomly fail, for a variety of reasons.
131+
132+
Sometimes a flake resolves itself after changes are pushed to re-trigger
133+
the checks. Consider [performing a rebase](#ciyaml-validation) to include
134+
the latest changes from the main branch.
135+
136+
Flakes often happen due to **infra errors**.
137+
For information on how to view and report infrastructure bugs, see the
138+
[infra failure overview](../../infra/Understanding-a-LUCI-build-failure.md#overview-of-an-infra-failure-build).

docs/infra/Understanding-a-LUCI-build-failure.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,5 @@ Please refer to the above example of the infra failure.
5151
4. Check if a flaky bug has been filed in [the flaky issues list](https://github.com/flutter/flutter/issues?q=is%3Aopen+is%3Aissue+label%3A%22c%3A+flake%22)
5252
5. File a new bug if needed
5353
6. If a rerun is needed, please refer to step 6 in the above infra failure session.
54+
55+
See also: [How to fix a PR's failing checks](../contributing/testing/Fix-failing-checks.md)

0 commit comments

Comments
 (0)