Details

Reviewers

deadalnix
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGING7765a25a6db4: [CI] Refactor the build configuration by moving out tests from the build
rABC7765a25a6db4: [CI] Refactor the build configuration by moving out tests from the build

Summary

This makes it easier to individually add and configure each step for
each configuration, and makes the whole build definition easier to read.

As a (good) side effect the functional tests are no longer run with the next
upgrade enabled with the asan and ubsan configurations, and use the
standard cutoff.

Depends on D3992 and D4014.

Test Plan

Run the CI builds/IBD and check the builds are green (ubsan excepted).

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Sep 4 2019, 15:33

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 4 2019, 15:33

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7315: Diff 11080.Sep 4 2019, 15:40

Fabien added a child revision: D3994: [CI] Workaround ubsan failure in functional tests.Sep 4 2019, 15:41

Remove setup.sh, now it is a function in build-configurations.sh.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing failed.

Harbormaster failed remote builds in B7319: Diff 11086!Sep 4 2019, 19:38

Move to the build dir *AFTER* its creation...

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7320: Diff 11087.Sep 4 2019, 19:48

jasonbcox requested changes to this revision.Sep 4 2019, 20:19

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

contrib/teamcity/build-configurations.sh
28 ↗	(On Diff #11087)	double comment ##
30 ↗	(On Diff #11087)	This can be collapsed into the line above it.
57 ↗	(On Diff #11087)	It's unnecessary to pass this value into the function only to export it again. it's probably best to define it at the top of the script.
61 ↗	(On Diff #11087)	typo: `test.s`
84 ↗	(On Diff #11087)	Food for thought: This sort of improvement could be done in a separate diff and make the review easier. I won't block this diff on this change, but just keep in mind for the future. There are other small (mostly one-liner) changes that could have had the same fate.
contrib/teamcity/build.sh
14 ↗	(On Diff #11087)	Removing these defaults will break IBD tests (IBD hasn't been migrated to build-configurations yet, so the config is still partially managed by TeamCity).

This revision now requires changes to proceed.Sep 4 2019, 20:19

Build Bitcoin-ABC / Resource Intensive Builds / Bitcoin-ABC IBD Diff Testing (w/o checkpoints) started.

Build Bitcoin-ABC / Resource Intensive Builds / Bitcoin-ABC IBD Diff Testing (w/o checkpoints) failed.

Fabien added inline comments.Sep 6 2019, 12:56

contrib/teamcity/build-configurations.sh
30 ↗	(On Diff #11087)	Doing so will discard the return value of the command line in favor of the return value of `export`, and cause a linter error.
84 ↗	(On Diff #11087)	I'm not sure what improvement you're talking about, but using `test_runner` cannot really be separated from the global move as it requires the environment to be compliant with how the test is executed. Splitting the `setup` function would have break the `build` script, and the opposite is true as well. I your comment is not about this then please ignore mine.

Fabien edited the summary of this revision. (Show Details)Sep 6 2019, 12:56

Fabien edited the test plan for this revision. (Show Details)

Fabien added a parent revision: D4014: [CI] Make IBD a standard build configuration.

Rebase on top of D4014 and address feedbacks.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7342: Diff 11122.Sep 6 2019, 13:48

jasonbcox requested changes to this revision.Sep 12 2019, 21:44

jasonbcox added inline comments.

contrib/teamcity/ibd.sh
1 ↗	(On Diff #11122)	This file needs another rebase, sorry.
17 ↗	(On Diff #11122)	Rather than removing TOPLEVEL and not defining BUILD_DIR, provide reasonable defaults: : "${TOPLEVEL:=$(git rev-parse --show-toplevel)}" : "${BUILD_DIR:=${TOPLEVEL}/build}" This preserves the ability to run this script directly without any setup.