OmniLayer / OmniJ

OmniLayer for Java, JVM, and Android
Apache License 2.0
133 stars 89 forks source link

CI: Bitcoin Core Boost + RegTests #33

Closed dexX7 closed 3 years ago

dexX7 commented 9 years ago

@msgilligan: I was wondering: are the tests in ./src/test/test_bitcoin and ./src/qt/test_bitcoin-qt executed when testing Master Core? There is also a Java RPC test suite which can be included directly into the build process and configuration: https://github.com/TheBlueMatt/test-scripts

A build script could probably look like this:

JAVA_COMPARISON_TOOL_URL=https://github.com/TheBlueMatt/test-scripts/blob/38b490a2599d422b12d5ce8f165792f63fd8f54f/pull-tests-0f7b5d8.jar?raw=true
JAVA_COMPARISON_TOOL_HASH=ecd43b988a8b673b483e4f69f931596360a5e90fc415c75c4c259faa690df198

# setup dependencies etc.
curl -k -L -o ./share/BitcoindComparisonTool.jar $JAVA_COMPARISON_TOOL_URL
if [[ "$(sha256sum ./share/BitcoindComparisonTool.jar)" != "$JAVA_COMPARISON_TOOL_HASH  ./share/BitcoindComparisonTool.jar" ]]; then echo "Hashes don't match."; exit 1; fi

# build
./autogen.sh
./configure --with-comparison-tool=./share/BitcoindComparisonTool.jar
make

# IIRC required for the bitcoind-comparison-test
cp ./src/mastercored ./src/bitcoind
cp ./src/mastercore-cli ./src/bitcoin-cli
make check

This downloads the Java test file and builds Bitcoin/Master Core with "test awareness". make check then executes the Boost tests (test_bitcoin, test_bitcoin_qt) as well as the bitcoind-comparison-test.

msgilligan commented 9 years ago

The CI build does not (currently) build the Qt version of Bitcoin. The Jenkins Job omnicore-integration runs the following "Execute Shell" command:

./autogen.sh
./configure
make
./src/test/test_bitcoin --log_format=XML --log_sink=test_bitcoin.xml --log_level=test_suite --report_level=no
msgilligan commented 9 years ago

Depending upon how long the Java Comparison Tool takes to run, we could either add it directly to the Omni Core builds or (as we do for the other integration tests) make it a "Downstream Project" and trigger it automatically after the completion of the omnicore-integration and omnicore-stable builds.

dexX7 commented 9 years ago

Takes about 1-2 minutes and creates a bunch of debug output which could be captured.

dexX7 commented 9 years ago

Yesterday I tested the integration of bitcoin-spock with Travis and Circle CI and noticed the bash scripts could need an update, e.g. I'm not sure what's the purpose of regtest-send-many.sh. Furthermore it looks like run-regtest.sh basically can't be used with Master/Omni Core out of the box. bitcoin.conf seems outdated and I'm not even sure, if it could be used as all, since the server=1 as well as regtest=1 parameters are missing. Wildcard use such as rpcallowip=* won't be allowed in Core 0.10, and should be replaced by rpcallowip=127.0.0.1.

If I'm not mistaken, then Matt's Java tests use run-bitcoind-for-test.sh.in to start the client where placeholder variables are replaced during making. The new RPC tests of Bitcoin Core 0.10 (as well as the derived mastercore-rpc-tests) handle node startup on their own and basically only need to know where the files are located (e.g. ../../src when starting from ./qa/rpc-tests). Results that would be written to .bitcoin/regtest end up in /tmp/testRANDOM and deleted after execution, unless optionally kept.

Long story short: could you maybe remove the files that are currently not or no longer in use? Then I suggest we think about an updated version, and I'd like to do something like:

git clone -b mscore-0.0.9 https://github.com/mastercoin-MSC/mastercore.git mastercore-0.0.9
cd mastercore-0.0.9/
./autogen.sh
./configure
make
make check
git clone https://github.com/msgilligan/bitcoin-spock.git qa/mastercore-spock-tests
./qa/mastercore-spock-tests/run-regtests.sh

While writing, I figured out it's possible to run tasks from another dir, which I was wondering about yesterday.. the documentation helps.. :) So maybe, and I'm just brainstorming, instead of the above, it may make more sense to do something like:

./qa/mastercore-spock-tests/start-in-regtest-mode.sh --daemon=./src/mastercored --cli=./src/mastercore-cli --log-output=./qa/logs
./qa/mastercore-spock-tests/gradlew -b qa/mastercore-spock-tests/build.gradle groovydoc
./qa/mastercore-spock-tests/gradlew -b qa/mastercore-spock-tests/build.gradle runRegtest

Edit: in case you're interested: .travis.yml (results), circle.yml (results + hosted test report)

msgilligan commented 9 years ago

The purpose of regtest-send-many.sh was to demonstrate the Bitcoin Core account balance handing issues with nothing but a BASH script.

Commit aa6933004ba21e82ad75aa4d0f42144dee88e747 removes some out-of-date scripts and moves some others to src/scripts. The remaining 3 in the root are all called by Jenkins.

dexX7 commented 9 years ago

There are basically no policy checks for TMSC or testnet, and because consensus issues on mainnet/main ecosystem are highly critical and should ring all alarm bells, I suggest to test TMSC in a different job, to maximize visibility of real issues.

dexX7 commented 9 years ago

Just saw the failing tests on the CI server: http://ci.omni.foundation/job/msc-integ-regtest/235/

Do you clean the whole regtest dir before a start?

msgilligan commented 9 years ago

Yes, see https://github.com/msgilligan/bitcoin-spock/blob/master/test-msc-integ-regtest.sh#L27

dexX7 commented 9 years ago

Yesterday I setup another VM (which I unfortunally already deleted) and played around with Jenkins, it's GitHub plugin and the GitHub pull request builder.

Even though the configuration and possible options are very impressive, I was a bit frustrated, because the plugins in combination are okay, but not very well integrated, at least that was my first impression, but I basically had no idea what I was doing.. I had two goals, which I both achived, and I try to reconstruct the process, so it could be used here and for the other projects as well. I wouldn't be surprised, if there are alternatives or more optimal ways.

Status report for commits:

status_report

  1. Install the GitHub plugin.
  2. Within the application to test (e.g. bitcoin-spock), under Settings and Webhooks & Services, the plugin is available as service and should point to Jenkin's hook URL.
  3. Furthermore seems to be required that a project maintainer creates an OAuth API token in GitHub's Settings -> Applications -> Personal access tokens, with at least repo:status, and likely a other permissions. I'm not sure, what the minimum is here.
  4. In the Jenkins configuration this access token can be used to configure the GitHub plugin. If I recall correctly, then the option was Let Jenkins auto-manage hook URLs. For me it was required to provide an API access token for the plugin and to create a service hook in the project's settings at the same time, so that the status is set and a build triggered, when merging a commit.
  5. I then created a new job with the build trigger Build when a change is pushed to GitHub.
  6. As build step I selected something like Set GitHub commit status as "pending". This was added recently to the plugin, and doesn't seem to be available on the current CI server.
  7. Finally as post-build action Set build status on GitHub commit.

Status reports for pending pull requests

88f7902f4e7981acde8972f30fb7e797

  1. Install the GitHub pull request builder.
  2. In the Jenkins configuration for this plugin I submitted my user credentials (what the hell actually), and let the plugin generate an access token. The user I created the token for was also set as admin.
  3. I basically disabled all text or comment based feedback and reportings, because I didn't want comments, but simply status updates for PRs.
  4. On a job level, there is a build-trigger GitHub Pull Request Builder.
  5. In it's advanced settings a white-list can be defined, so that only white listed users actually trigger a build or test, when pushing a PR.

This is probably not a complete description, but roughly how I did it. Compared to Travis or Circle CI, the setup and configuration was pretty rough and time consuming, but achieving similar results is definitely possible, and it would be awesome, if we could adopt such practise.

I really hope we get to the point where basically everything is tested, and the PR review process (of Omni Core and others) becomes a lot easier, especially for PRs with a lot of line changes, which might not even touch any critical logic. I don't even want to imagine how many review time a trivial push such as using clang-format on a whole project would consume, if reviewed manually... hehe. ;)

msgilligan commented 9 years ago

Yeah, I would like to be able to use the pull request tester. I'll take a look at this. The white-list looks like a good idea. I'm pretty sure Travis uses Docker to provide security/isolation.

Another possibility is we could create separate master and develop branches. master would be used for actually testing/validating omnicore/bitcoind and develop would be used for working on the tests themselves. The two branch idea is worth considering independently of the PR tester as well.

dexX7 commented 9 years ago

I'll take a look at this.

This would be awesome, thanks! Please let me know, if I can help, but I believe you are much, much more familiar with Jenkins than I am.

Another possibility is we could create separate master and develop branches ...

Sorry, I'm unable to follow. While having a master and develop branch does make a lot of sense, e.g. to avoid creating the impression something is totally broken, such as consensus, because we screwed up the test framework.

But what's unclear to me is how this relates to a pull tester and setting GitHub statuses.

To be clear: while it would be awesome to have it here, and this is also where we should begin testing the waters, my endgame plan is for Omni Core and automated PR testing over there.

msgilligan commented 9 years ago

You're following fine, @dexX7 . The master and develop branches would help in the way that you understand, but are not a complete replacement for having a pull tester.

msgilligan commented 3 years ago

I believe this has been implemented in Omni Core. In any event it is not an OmniJ issue, per se.