Page MenuHomePhabricator

[CI] Fail fast the builds that print error to stdout
ClosedPublic

Authored by Fabien on Oct 17 2020, 18:27.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABCa09e74275319: [CI] Fail fast the builds that print error to stdout
Summary

Some builds print errors to stdout. As a consequence we can't jump a some specific test failure when an error is thrown, and the error message displayed to the user is the build tail.
This diff makes these build stop on first error, so that the build tail will contain the actual issue and the user won't need to look at the full teamcity build log.

Test Plan

Edit 2 files to remove some braces, then:

./contrib/teamcity/build-configurations.py build-clang-tidy

Check the build stops after the first missing brace.

./contrib/teamcity/build-configurations.py build-clang-asan
./contrib/teamcity/build-configurations.py build-clang-ubsan
./contrib/teamcity/build-configurations.py build-clang-tsan

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Oct 17 2020, 18:27
deadalnix requested changes to this revision.Oct 17 2020, 19:04
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
contrib/teamcity/build-configurations.yml
82 ↗(On Diff #24762)

This does exactly nothing.

This revision now requires changes to proceed.Oct 17 2020, 19:04
Fabien retitled this revision from [CI] Stop on first failure when running the clang-tidy build to [CI] Fail fast the builds that print error to stdout.Oct 19 2020, 09:01
Fabien edited the summary of this revision. (Show Details)
Fabien edited the test plan for this revision. (Show Details)
Fabien edited the test plan for this revision. (Show Details)

Add an explicit fail_fast option, extend to sanitizers

deadalnix requested changes to this revision.Oct 19 2020, 16:17
deadalnix added inline comments.
contrib/teamcity/build-configurations.py
191 ↗(On Diff #24775)

Well that's your problem. You want this on test suite running on master, but not on the one running on each diff.

You can solve this whole thing durably by using sensible defaults.

This revision now requires changes to proceed.Oct 19 2020, 16:17

Pick better default so the builds running on diff will always fail fast.

Why do sanitizers need to fail fast on master?

The sanitizer output is printed to console (we use to have it in a log file but it was difficult to debug because it missed the context of when it ran, and the stack is sometimes a bit cryptic).
When a sanitizer issue occurs, you might need to scroll through all the log to find the printed issue. By making it fail fast, the error will be located close to the end of the log and easy to spot.

This revision is now accepted and ready to land.Oct 21 2020, 23:18