Differential D7993

test: Remove duplicate NodeContext hacks
ClosedPublic
Actions

Authored by Fabien on Oct 19 2020, 22:03.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABCc146a4640bf9: test: Remove duplicate NodeContext hacks

Summary

Qt tests currently are currently using two NodeContext structs at the
same
time, one in interfaces::NodeImpl::m_context, and the other in
BasicTestingSetup::m_node, and the tests have hacks transferring state
between
them.

Fix this by getting rid of the NodeImpl::m_context struct and making it
a
pointer. This way a common BitcoinApplication object can be used for all
qt
tests, but they can still have their own testing setups.

Non-test code is changing but non-test behavior is still the same as
before.

Motivation for this PR is to be able to remove the
"std::move(test.m_node.connman)" and mempool hacks for swapping
individual
NodeContext members in Qt tests, because followup PR #19099 adds yet
another
member (wallet_client) that needs to be swapped. After this change, the
whole
NodeContext struct can be swapped instead of individual members, so the
workarounds are less fragile and invasive.

Backport of core PR19098.

Depends on D7992.

Test Plan

ninja all check-all
./contrib/teamcity/build-configurations.py build-ubsan

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Oct 19 2020, 22:03

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 19 2020, 22:03

Fabien requested review of this revision.Oct 19 2020, 22:03

teamcity edited the summary of this revision. (Show Details)Oct 19 2020, 22:03

@bot build-asan

Harbormaster completed remote builds in B13503: Diff 24803.Oct 19 2020, 22:04

Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

  -t abc-base-image-"${HASH}" debian:buster

(cat <<EOF
results() {
  set +e
  shopt -s dotglob nullglob
  mv /work/abc-ci-builds/"build-debug"/* /results
  chown -R ${ME} /work
  chown -R ${ME} /results
  chown -R ${ME} /root/.ccache
}
trap "results" EXIT
export TEAMCITY_VERSION="2019.2.4 (build 72059)"
export BASE_CACHE="/root/abc-depends/cache"
export SDK_ARCHIVE_DIR="/root/abc-depends/osx-sdk"
export SOURCES_PATH="/root/abc-depends/sources"
mkdir -p "/root/abc-depends/cache" "/root/abc-depends/osx-sdk" "/root/abc-depends/sources"
./contrib/teamcity/build-configurations.py "build-debug"
EOF
) > run-command.sh
chmod +x run-command.sh

~/infra/docker/docker-run.sh \
  -a "-v /home/teamcity/.ccache:/root/.ccache -v /home/teamcity/.abc-depends:/root/abc-depends -v "${RESULTS_DIR}":/results" \
  -c run-command.sh /work/run-command.sh abc-base-image-"${HASH}" ./run-command.sh

[22:03:48] :	 [Step 1/1] Starting: /home/teamcity/buildAgent/temp/agentTmp/custom_script6275367189717703451
[22:03:48] :	 [Step 1/1] in directory: /home/teamcity/buildAgent/work/jailed-build
[22:03:48] :	 [Step 1/1] ~/buildAgent/work/jailed-build/bitcoin-abc ~/buildAgent/work/jailed-build
[22:03:48] :	 [Step 1/1] ~/buildAgent/work/jailed-build
[22:03:48] :	 [Step 1/1] Building base image for: 80c9b066e...
[22:03:48] :	 [Step 1/1] ~/buildAgent/work/jailed-build/bitcoin-abc ~/buildAgent/work/jailed-build
[22:03:50] :	 [Step 1/1] ~/buildAgent/work/jailed-build
[22:03:50] :	 [Step 1/1] Tag name: abc-base-image-80c9b066e
[22:04:07]W:	 [Step 1/1] Traceback (most recent call last):
[22:04:07]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 570, in <module>
[22:04:07]W:	 [Step 1/1]     main()
[22:04:07]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 559, in main
[22:04:07]W:	 [Step 1/1]     script_dir, config_path, args.build)
[22:04:07]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 59, in __init__
[22:04:07]W:	 [Step 1/1]     self.load(build_name)
[22:04:07]W:	 [Step 1/1]   File "./contrib/teamcity/build-configurations.py", line 82, in load
[22:04:07]W:	 [Step 1/1]     self.name, list(config.keys())
[22:04:07]W:	 [Step 1/1] AssertionError: build-debug is not a valid build identifier. Valid identifiers are ['templates', 'builds']
[22:04:07]W:	 [Step 1/1] mv: missing destination file operand after '/results'
[22:04:07]W:	 [Step 1/1] Try 'mv --help' for more information.
[22:04:11]W:	 [Step 1/1] Process exited with code 1
[22:04:11]E:	 [Step 1/1] Process exited with code 1 (Step: Command Line)
[22:04:11]E:	 [Step 1/1] Step Command Line failed
[22:04:11]E: Ant JUnit report watcher
[22:04:11]E:	 [Ant JUnit report watcher] No reports found for paths:
[22:04:11]E:	 [Ant JUnit report watcher] +:results/test_bitcoin.xml
[22:04:11]E:	 [Ant JUnit report watcher] +:results/**/junit_results*.xml
[22:04:11] : Publishing internal artifacts (4s)
[22:04:16] :	 [Publishing internal artifacts] Publishing 1 file using [WebPublisher]
[22:04:16] :	 [Publishing internal artifacts] Publishing 1 file using [ArtifactsCachePublisher]
[22:04:11]W: Publishing artifacts (5s)
[22:04:11] :	 [Publishing artifacts] Collecting files to publish: [+:results/**/junit_results*.xml]
[22:04:11]W:	 [Publishing artifacts] Artifacts path 'results/**/junit_results*.xml' not found
[22:04:17] : Build finished

Rebase for CI

Harbormaster completed remote builds in B13504: Diff 24804.Oct 19 2020, 22:29

deadalnix requested changes to this revision.Oct 19 2020, 23:56

deadalnix added a subscriber: deadalnix.

deadalnix added inline comments.

src/interfaces/node.cpp
129 ↗	(On Diff #24804)	This takes a context in the source material. Can you either backport this dependency or add a note?

This revision now requires changes to proceed.Oct 19 2020, 23:56

Fabien added inline comments.Oct 20 2020, 15:13

src/interfaces/node.cpp
129 ↗	(On Diff #24804)	I started backporting the dependencies but it pulling a lot of refactors and will take some time to complete, so I'd better make this move forward to unbreak ASAN. The changes are unrelated to this diff so it will not conflict, and the type checking will prevent from introducing an error.