Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGING9069cd7940fc: Make all automated commits run smoke tests prior to pushing
rABC9069cd7940fc: Make all automated commits run smoke tests prior to pushing

Summary

It's a good idea to make sure that the happy path build is green and some unit tests and functional tests pass.
It's not feasible to test everything, as we don't want automatic commits to take a long time, otherwise we risk
merge conflicts. But, we can be reasonably confident that nothing is catastrophically broken, and CI will pick up the
rest once the change is detected by Teamcity.

Depends on D5269

Test Plan

See D5269

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jasonbcox created this revision.Feb 12 2020, 00:38

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 12 2020, 00:38

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9451: Diff 16310.Feb 12 2020, 00:55

Fabien requested changes to this revision.Feb 13 2020, 10:03

Fabien added a subscriber: Fabien.

Fabien added inline comments.

contrib/source-control-tools/automated-commits.sh
109 ↗	(On Diff #16310)	Smoke tests is not very meaningful to someone editing the `build-configurations.sh` script. `build-automated-commits-tests` or similar is more relevant.
contrib/teamcity/build-configurations.sh
203 ↗	(On Diff #16310)	Thinking more about this it might not even be at the right place. What is the point of having an new build type in the CI, that reports status (what does the status mean ?) for automated commits ?

This revision now requires changes to proceed.Feb 13 2020, 10:03

jasonbcox added inline comments.Feb 13 2020, 20:29

contrib/source-control-tools/automated-commits.sh
109 ↗	(On Diff #16310)	This is true, but limiting the scope of those tests doesn't seem necessary when that build config can be used in other places in the future. See my other comment.
contrib/teamcity/build-configurations.sh
203 ↗	(On Diff #16310)	The intent was this could be reused in places other than just automated commits, but it does not need to run in CI on every commit. The other existing builds that complete rather quickly (just a couple minutes) do not have good general test coverage because they're focusing on something specific (build-without-wallet, build-werror, etc). IMO the name `build-smoke-tests` is self explanatory when thought about in a general context.

jasonbcox mentioned this in D5291: Move build_* wrapper scripts to devtools.Feb 14 2020, 20:06

jasonbcox mentioned this in rABC1938396c2de4: Move build_* wrapper scripts to devtools.Feb 18 2020, 22:04

Revised design for smoke tests:

Decouple from build-configurations.sh since smoke tests do not have a direct relationship to CI.
Smoke tests do not need to live in their own script since they are not universally applicable anyway.
Leverage build_cmake which was recently moved out of contrib/teamcity (since it's not CI-specific).