Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: extended logging on expect test abort #4343

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Aug 2, 2022

Moved logs printing from WaitForRound to Abort method.

Sometimes expect tests fail with messages like this that do not help at all.

** Error getting status: Get "http://127.0.0.1:45095/v1/status": dial tcp 127.0.0.1:45095: connect: connection refused **
Aborting with Error: error getting network status: 

Now test output has more details.

Copy link
Contributor

@algonautshant algonautshant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.
I don't see any problems with this. This is a useful fix.

@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #4343 (2215ac9) into master (82b7529) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4343   +/-   ##
=======================================
  Coverage   55.23%   55.24%           
=======================================
  Files         395      395           
  Lines       50351    50351           
=======================================
+ Hits        27813    27817    +4     
+ Misses      20145    20143    -2     
+ Partials     2393     2391    -2     
Impacted Files Coverage Δ
ledger/tracker.go 71.79% <0.00%> (-3.85%) ⬇️
util/db/dbutil.go 49.09% <0.00%> (-1.22%) ⬇️
data/abi/abi_type.go 87.67% <0.00%> (-0.95%) ⬇️
network/wsNetwork.go 64.89% <0.00%> (ø)
network/wsPeer.go 68.21% <0.00%> (+0.27%) ⬆️
cmd/tealdbg/debugger.go 73.49% <0.00%> (+0.80%) ⬆️
data/transactions/verify/txn.go 44.88% <0.00%> (+0.88%) ⬆️
crypto/merkletrie/node.go 93.48% <0.00%> (+1.86%) ⬆️
agreement/proposalManager.go 98.03% <0.00%> (+1.96%) ⬆️
agreement/cryptoVerifier.go 69.71% <0.00%> (+2.11%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@algorandskiy algorandskiy merged commit 5f62166 into algorand:master Aug 2, 2022
@@ -50,11 +50,41 @@ proc ::AlgorandGoal::Abort { ERROR } {
# terminate child algod processes, if there are active child processes the test will hang on a test failure
puts "GLOBAL_TEST_ROOT_DIR $::GLOBAL_TEST_ROOT_DIR"
puts "GLOBAL_NETWORK_NAME $::GLOBAL_NETWORK_NAME"

log_user 1
set NODE_DATA_DIR $GLOBAL_TEST_ROOT_DIR/Primary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if Primary and Node are not the names used by a test?
WaitForRound gets the node dir as param: { WAIT_FOR_ROUND_NUMBER NODE_DATA_DIR
which takes care of this possibility, but not here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants