dart-lang / dart_ci

Tools used by Dart's continuous integration (CI) testing that aren't needed by Dart SDK contributors. Mirrored from dart.googlesource.com/dart_ci. Do not land pull requests on Github.
BSD 3-Clause "New" or "Revised" License
18 stars 5 forks source link

Aggregated test results for CL should link to correct logs for flaky tests. #160

Open mraleph opened 9 months ago

mraleph commented 9 months ago

I see the following in the CL results:

Screenshot 2023-11-28 at 12 52 20

However hoovering over flaky tests does not give me any actionable information about failed runs of those tests. The log link does not take me to the log which shows the failure that occured.

This is very frustrating user experience. How do I see actual failure logs?

mraleph commented 9 months ago

/cc @athomas @whesse

whesse commented 9 months ago

In this case, the tests are also flaky on the CI, so clicking on the history link gives a link to a CI build with the failure, and a working link to the logs server that shows the log for that specific test. The flakiness dashboard also has links to the failure logs for flaky tests.

The logs for flaky tests are completely dropped from try builds. The results.json and logs.json are not uploaded to cloud storage, a subset of logs.json for only the new non-flaky failures is reported, and the full logs are not saved or output anywhere.

The individual logs.json from each shard can be seen, but there are 20 shards that would need to be looked at individually. We could add a step that just outputs the combined logs.json, the same way that step "add fields to result records" has a link to results.json.

The step could also instead print out the logs in a more human-readable form, since the json encoded logs.json has the newlines escaped in each test log.

The only case where these logs, for new flaky tests, would be needed is when the CL truly introduces new flakiness, that was not present before, so the history links don't show the flakiness as already existing, with failure logs available.

mraleph commented 6 months ago

I am hitting this again. I have flaky test - and I have no way to see the logs.

Screenshot 2024-03-11 at 14 24 54

This is a very bad UX /cc @athomas

mraleph commented 6 months ago

Here is the suggestion (idea from @mkustermann) which should be an easy update to the recipes: deflaking should always run without swallowing logs. This way I can at least inspect the deflaking step to understand what is going wrong. Maybe just set --progress=verbose or something like that?

whesse commented 6 months ago

The test failures should appear in the stdout of deflaking steps after landing https://dart-review.googlesource.com/c/recipes/+/356820

whesse commented 6 months ago

@sigmundch

mraleph commented 6 months ago

@whesse is it possible to also make a change to the step which does "list tests to deflake" so that it also prints logs of failed tests which trigger deflaking?

Because I assume test could fail on the shard but then not fail at all on deflaking.

whesse commented 6 months ago

There is no good reason why we haven't been running the "find ignored flaky test failure logs" step on try jobs, and running that step will put the failure log in a link on the test results step, whether it is in the first run or deflaking run.

The only think missing is that this step will only print one failure log for each test - it is built in to the compare_results script in the SDK that it only extracts one result and log per test name/configuration pair. But that is OK for this case, where you just need the failure log wherever it occurs.

Alternatively, we could always make a logs.json link, the way that we make a results.json link in the "add fields to results" step. Having links in the builder steps is easier than uploading the logs to cloud storage, because we don't have a bucket, lifetime policy, or directory scheme for tryjob results, and would need to make it.

But I think adding the "ignored flaky test failure logs" may be sufficient. https://dart-review.googlesource.com/c/recipes/+/357040