Skip to content

tests: Fix account tests for calling pending txn endpoint and box tests#4751

Merged
michaeldiamant merged 2 commits intoalgorand:masterfrom
algochoi:fix-pendingtxn-tests
Nov 3, 2022
Merged

tests: Fix account tests for calling pending txn endpoint and box tests#4751
michaeldiamant merged 2 commits intoalgorand:masterfrom
algochoi:fix-pendingtxn-tests

Conversation

@algochoi
Copy link
Copy Markdown
Contributor

@algochoi algochoi commented Nov 3, 2022

Summary

This PR changes some existing tests that call the GetPendingTransactions endpoint to use the v2 model instead of the v1 model. Also changes the goal box command to account for client-side error string handling.

Test Plan

Integration tests.

@algochoi algochoi changed the title Fix account tests for calling pending txn endpoint tests: Fix account tests for calling pending txn endpoint Nov 3, 2022
@algochoi algochoi marked this pull request as ready for review November 3, 2022 21:48
@algochoi algochoi changed the title tests: Fix account tests for calling pending txn endpoint tests: Fix account tests for calling pending txn endpoint and box tests Nov 3, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 3, 2022

Codecov Report

Merging #4751 (1f6e037) into master (06d146b) will increase coverage by 0.03%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #4751      +/-   ##
==========================================
+ Coverage   54.53%   54.56%   +0.03%     
==========================================
  Files         414      414              
  Lines       53607    53607              
==========================================
+ Hits        29234    29253      +19     
+ Misses      21945    21929      -16     
+ Partials     2428     2425       -3     
Impacted Files Coverage Δ
cmd/goal/box.go 21.62% <0.00%> (ø)
ledger/blockqueue.go 85.63% <0.00%> (-2.88%) ⬇️
catchup/peerSelector.go 98.95% <0.00%> (-1.05%) ⬇️
data/transactions/verify/txn.go 76.19% <0.00%> (-0.96%) ⬇️
network/wsNetwork.go 65.52% <0.00%> (+0.18%) ⬆️
catchup/service.go 68.64% <0.00%> (+0.49%) ⬆️
ledger/testing/randomAccounts.go 57.23% <0.00%> (+0.61%) ⬆️
crypto/merkletrie/node.go 93.48% <0.00%> (+1.86%) ⬆️
network/wsPeer.go 68.44% <0.00%> (+1.94%) ⬆️
crypto/merkletrie/trie.go 68.61% <0.00%> (+2.18%) ⬆️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@michaeldiamant michaeldiamant left a comment

Choose a reason for hiding this comment

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

@algochoi Thanks for the fix - Merging now to address failed build: https://app.circleci.com/pipelines/github/algorand/go-algorand/10341/workflows/c4dac869-0f91-4ce1-8b7d-abd0e5e63ea9.

@jasonpaulos I request that you still perform a review to identify pending threads.

@michaeldiamant michaeldiamant merged commit ae443cd into algorand:master Nov 3, 2022
@algochoi algochoi deleted the fix-pendingtxn-tests branch November 3, 2022 23:08
Copy link
Copy Markdown
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Changes look good to me

@algorandskiy
Copy link
Copy Markdown
Contributor

@algochoi @michaeldiamant looks like there is another one in e2e subs box-search.sh:

+ '[' 'Error processing command: HTTP 404 Not Found: box not found' = 'No box found for appid 75 with name str:not_found' ']'

@jasonpaulos
Copy link
Copy Markdown
Contributor

@algorandskiy that failure was addressed by the change to cmd/goal/box.go in this PR. The build now passes on master.

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.

4 participants