search

dtr-org / unit-e

A digital currency for a new era of decentralized trust
https://unit-e.io
MIT License
45 stars 15 forks source link

unit tests can no longer be run in parallel #861

Closed scravy closed 5 years ago

scravy commented 5 years ago

Describe the bug https://github.com/dtr-org/unit-e/pull/793 (557f08c19004fccdf7ddf8f4030eaec38f77a829) introduced a non-acceptable regression: Unit tests can no longer be run in parallel.

We have a script that runs unit tests independently as separate processes and in parallel in contrib/devtools/run-unit-tests.sh. I've binary searched through the commit history and built until I arrived at 557f08c19004fccdf7ddf8f4030eaec38f77a829 which apparently breaks this.

I've verified:

74d598a2f3cecbd138aa02bf1c1e02ea2d548b7a – okay a8a002f274acb9b07172986c6e87997c97e777e4 – okay ba3ebd4e50324292c4f15eaaa1783a9837a703bd – broken 557f08c19004fccdf7ddf8f4030eaec38f77a829 – broken cc3bb51638a2e3dc756412f055aae92ae305a467 (parent of 557f08c19004fccdf7ddf8f4030eaec38f77a829) – okay

To Reproduce Run contrib/devtools/run-unit-tests.sh.

Expected behavior All tests should pass.

Environment Ubuntu 18.04 using gcc, macOS using clang

Additional context none.

frolosofsky commented 5 years ago

My best guess was that the problem happens to one of the newly added tests which could depend on a global context somehow. I ran every new test independently; they pass.

Unfortunately, the way how the script reports errors is super uninformative. It finished with zero code, and it has only one test-suite.log file

# TOTAL: 1
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0

It's impossible to investigate this issue without having appropriate logging. I suggest to improve contrib/devtools/run-unit-tests.sh first.

scravy commented 5 years ago

The current build definitions do not run the unit tests in parallel using make check, which is why the blame is on this little helper script. But in bitcoin they did add running unit tests in parallel and we will get it with the merge of 0.17 too: https://github.com/bitcoin/bitcoin/pull/12926

So this issue is about the unit tests having been mutilated to not have the property of being independent anymore.

I am taking care of fixing this. Future issues should be detectable by the improved build definitions once https://github.com/dtr-org/unit-e/pull/860 is good to go.

scravy commented 5 years ago

I pushed the discussed improvement to run-unit-tests.sh. It now properly propagates exit code / failure and saves the log outputs for the failing tests only: https://github.com/dtr-org/unit-e/pull/864