Page MenuHomePhabricator
Differential D9636

[testing] enable '-ecash' only on tests that use it
AbandonedPublic
Actions

Authored by majcosta on Jun 6 2021, 08:53.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Summary

it was a mistake to enable it for all of them, this will hopefully make upcoming patches easier to review and also makes for slightly better test coverage

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
enable_ecash_selectively_functional_tests
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 15920
Build 31740: Build Diffbuild-debug · build-without-wallet · build-diff · build-clang-tidy · build-clang
Build 31739: arc lint + arc unit

Event Timeline

majcosta created this revision.Jun 6 2021, 08:53
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 6 2021, 08:53
majcosta requested review of this revision.Jun 6 2021, 08:53
Harbormaster completed remote builds in B15914: Diff 28770.Jun 6 2021, 09:01
teamcity added a comment.Jun 6 2021, 09:01
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: feature_notifications.py ======

------- Stdout: -------
2021-06-06T08:59:28.647000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_085758/feature_notifications_512
2021-06-06T08:59:28.967000Z TestFramework (INFO): test -blocknotify
2021-06-06T08:59:28.968000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/feature_notifications.py", line 66, in run_test
    else ADDRESS_BCHREG_UNSPENDABLE
  File "/work/test/functional/test_framework/coverage.py", line 48, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/work/test/functional/test_framework/authproxy.py", line 159, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: Error: Invalid address (-5)
2021-06-06T08:59:29.018000Z TestFramework (INFO): Stopping nodes
2021-06-06T08:59:29.220000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_085758/feature_notifications_512
2021-06-06T08:59:29.221000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_085758/feature_notifications_512/test_framework.log
2021-06-06T08:59:29.221000Z TestFramework (ERROR): 
2021-06-06T08:59:29.221000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_085758/feature_notifications_512' to consolidate all logs
2021-06-06T08:59:29.221000Z TestFramework (ERROR): 
2021-06-06T08:59:29.221000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-06-06T08:59:29.221000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-06-06T08:59:29.221000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: feature_notifications.py

majcosta updated this revision to Diff 28771.Jun 6 2021, 09:07

Depends on D9635

majcosta edited the summary of this revision. (Show Details)Jun 6 2021, 09:07
majcosta added a parent revision: D9635: [refactor] replace duplicated AmountFromValue function with Amount::Amount(UniValue T).
Harbormaster completed remote builds in B15915: Diff 28771.Jun 6 2021, 09:14
teamcity added a comment.Jun 6 2021, 09:14
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: feature_notifications.py ======

------- Stdout: -------
2021-06-06T09:13:24.722000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_091159/feature_notifications_495
2021-06-06T09:13:25.039000Z TestFramework (INFO): test -blocknotify
2021-06-06T09:13:25.040000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/feature_notifications.py", line 66, in run_test
    else ADDRESS_BCHREG_UNSPENDABLE
  File "/work/test/functional/test_framework/coverage.py", line 48, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/work/test/functional/test_framework/authproxy.py", line 159, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: Error: Invalid address (-5)
2021-06-06T09:13:25.090000Z TestFramework (INFO): Stopping nodes
2021-06-06T09:13:25.392000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_091159/feature_notifications_495
2021-06-06T09:13:25.392000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_091159/feature_notifications_495/test_framework.log
2021-06-06T09:13:25.392000Z TestFramework (ERROR): 
2021-06-06T09:13:25.393000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210606_091159/feature_notifications_495' to consolidate all logs
2021-06-06T09:13:25.393000Z TestFramework (ERROR): 
2021-06-06T09:13:25.393000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-06-06T09:13:25.393000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-06-06T09:13:25.393000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: feature_notifications.py

majcosta updated this revision to Diff 28772.Jun 6 2021, 09:27

fixed feature_notifications.py without wallet

Harbormaster completed remote builds in B15916: Diff 28772.Jun 6 2021, 09:31
teamcity added a comment.Jun 6 2021, 09:31
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Tail of the build log:

wallet_labels.py                        | ○ Skipped | 0 s
wallet_listreceivedby.py                | ○ Skipped | 0 s
wallet_listsinceblock.py                | ○ Skipped | 0 s
wallet_listtransactions.py              | ○ Skipped | 0 s
wallet_multiwallet.py                   | ○ Skipped | 0 s
wallet_multiwallet.py --usecli          | ○ Skipped | 0 s
wallet_reorgsrestore.py                 | ○ Skipped | 0 s
wallet_resendwallettransactions.py      | ○ Skipped | 0 s
wallet_txn_clone.py                     | ○ Skipped | 0 s
wallet_txn_clone.py --mineblock         | ○ Skipped | 0 s
wallet_txn_doublespend.py               | ○ Skipped | 0 s
wallet_txn_doublespend.py --mineblock   | ○ Skipped | 0 s
wallet_watchonly.py                     | ○ Skipped | 0 s
wallet_watchonly.py --usecli            | ○ Skipped | 0 s
wallet_zapwallettxes.py                 | ○ Skipped | 0 s

ALL                                     | ✓ Passed  | 448 s (accumulated) 
Runtime: 90 s

----------------------------------------------------------------------
Ran 5 tests in 0.002s

OK

[27/413] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.004s

OK
[28/413] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[126/413] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from /usr/include/boost/test/unit_test.hpp:19,
                 from ../../src/test/script_tests.cpp:30:
../../src/test/script_tests.cpp: In member function ‘void script_tests::script_build::test_method()’:
../../src/test/script_tests.cpp:540:22: note: variable tracking size limit exceeded with -fvar-tracking-assignments, retrying without
 BOOST_AUTO_TEST_CASE(script_build) {
                      ^~~~~~~~~~~~
[170/413] Running seeder test suite
PASSED: seeder test suite
[183/413] avalanche: testing processor_tests
FAILED: src/avalanche/test/CMakeFiles/check-avalanche-processor_tests 
cd /work/abc-ci-builds/build-without-wallet/src/avalanche/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-without-wallet/test/log/avalanche-processor_tests.log /work/abc-ci-builds/build-without-wallet/src/avalanche/test/test-avalanche --run_test=processor_tests --logger=HRF,message:JUNIT,message,avalanche-processor_tests.xml --catch_system_errors=no
Running 11 test cases...

*** No errors detected
Segmentation fault (core dumped)
[385/413] Running pow test suite
PASSED: pow test suite
[387/413] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[410/413] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1
majcosta updated this revision to Diff 28773.Jun 6 2021, 09:35

that flakiness in script_tests.cpp is unrelated, restarting build

Harbormaster completed remote builds in B15917: Diff 28773.Jun 6 2021, 09:35
majcosta updated this revision to Diff 28774.Jun 6 2021, 10:15

rebase

Harbormaster completed remote builds in B15918: Diff 28774.Jun 6 2021, 10:29
majcosta edited the summary of this revision. (Show Details)Jun 6 2021, 19:14
majcosta updated this revision to Diff 28777.Jun 6 2021, 19:15

rebase on master

Harbormaster completed remote builds in B15920: Diff 28777.Jun 6 2021, 19:29
Fabien requested changes to this revision.Jun 7 2021, 06:31
Fabien added a subscriber: Fabien.

Can you expand a bit of what the problem is from setting the flag globally ? Without more details it's hard to see how having the option repeated in and here in every test is better than setting it once for all.
Also if it increases the test coverage then the tests are wrong, because these tests are not testing the -ecash flag.

This revision now requires changes to proceed.Jun 7 2021, 06:31
majcosta abandoned this revision.Jun 8 2021, 21:01

you're right, this is not good

Revision Contents
Changeset List

Diff 28777

View Options

test/functional/abc_feature_minerfund.py

Loading...
View Options

test/functional/abc_mining_basic.py

Loading...
View Options

test/functional/abc_p2p_avalanche.py

Loading...
View Options

test/functional/feature_notifications.py

Loading...
View Options

test/functional/interface_zmq.py

Loading...
View Options

test/functional/p2p_inv_download.py

Loading...
View Options

test/functional/rpc_createmultisig.py

Loading...
View Options

test/functional/rpc_deriveaddresses.py

Loading...
View Options

test/functional/rpc_generateblock.py

Loading...
View Options

test/functional/rpc_invalidateblock.py

Loading...
View Options

test/functional/rpc_psbt.py

Loading...
View Options

test/functional/test_framework/test_node.py

Loading...
View Options

test/functional/wallet_balance.py

Loading...
View Options

test/functional/wallet_basic.py

Loading...
View Options

test/functional/wallet_importdescriptors.py

Loading...
View Options

test/functional/wallet_importmulti.py

Loading...