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

Prefer waiting over eventually in migration tests #2700

Merged
merged 2 commits into from
Jun 11, 2021

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jun 10, 2021

Issue Number

ADP-970, #2699

Overview

  • waitForTxImmutability should now only need to be 10s instead of 16s
  • Use waitForTxImmutability instead of eventually to protect against rollbacks in migration tests

Comments

  • An alternative solution would be to use the retrying it with eventually, but we should moving away from the retrying it, not re-adding it.

Our parameters must have changed since hard-coding the previous delay.
@Anviking Anviking force-pushed the anviking/ADP-970/waitForTxImmutability branch from a95fb39 to 1f81db0 Compare June 10, 2021 10:48
@Anviking Anviking self-assigned this Jun 10, 2021
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 10, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 10, 2021

try

Build failed:

#expected

@Anviking Anviking force-pushed the anviking/ADP-970/waitForTxImmutability branch from 1f81db0 to c302ef1 Compare June 10, 2021 11:05
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 10, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 10, 2021

try

Build failed:

Cardano.Wallet.DB.MVar
  MVar
    Extra Properties about DB initialization
building of '/nix/store/020dwr201i4z6s3xrrdlcr2ybbran40m-cardano-wallet-core-test-unit-2021.5.26-check' timed out after 900 seconds of silence

#2472

Mac

@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 10, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 10, 2021

try

Build failed:

  src/Test/Integration/Scenario/API/Shelley/Transactions.hs:335:22:
  1) API Specifications, SHELLEY_TRANSACTIONS, TRANS_CREATE_01x - Single Output Transaction
       While verifying (Status {statusCode = 200, statusMessage = "OK"},Right (ApiWallet {id = ApiT {getApiT = WalletId {getWalletId = ead1025d0402c19c08d32d8382db3dcff3a795be}}, addressPoolGap = ApiT {getApiT = AddressPoolGap {getAddressPoolGap = 20}}, balance = ApiWalletBalance {available = Quantity {getQuantity = 1869500}, total = Quantity {getQuantity = 1869500}, reward = Quantity {getQuantity = 0}}, assets = ApiWalletAssetsBalance {available = ApiT {getApiT = TokenMap (fromList [])}, total = ApiT {getApiT = TokenMap (fromList [])}}, delegation = ApiWalletDelegation {active = ApiWalletDelegationNext {status = NotDelegating, target = Nothing, changesAt = Nothing}, next = []}, name = ApiT {getApiT = WalletName {getWalletName = "Empty Wallet"}}, passphrase = Just (ApiWalletPassphraseInfo {lastUpdatedAt = 2021-06-10 13:02:43.829011 UTC}), state = ApiT {getApiT = Ready}, tip = ApiBlockReference {absoluteSlotNumber = ApiT {getApiT = SlotNo 2021}, slotId = ApiSlotId {epochNumber = ApiT {getApiT = EpochNo {unEpochNo = 20}}, slotNumber = ApiT {getApiT = SlotInEpoch {unSlotInEpoch = 21}}}, time = 2021-06-10 13:03:14.2 UTC, block = ApiBlockInfo {height = Quantity {getQuantity = 917}}}}))
       expected: Quantity {getQuantity = 0}
        but got: Quantity {getQuantity = 1869500}

  To rerun use: --match "/API Specifications/SHELLEY_TRANSACTIONS/TRANS_CREATE_01x - Single Output Transaction/"

  src/Test/Integration/Scenario/CLI/Shelley/Wallets.hs:754:18:
  2) CLI Specifications, SHELLEY_CLI_WALLETS, WALLETS_UTXO_02 - Utxo statistics works properly
       Waited longer than 90s to resolve action: "Wallet balance is as expected".
       expected: Quantity {getQuantity = 1562000000}
        but got: Quantity {getQuantity = 0}

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_WALLETS/WALLETS_UTXO_02 - Utxo statistics works properly/"

  src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:129:18:
  3) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_01x - Restoration from account public key preserves funds
       Waited longer than 90s to resolve action: "Wallet balance is as expected".
       expected: Quantity {getQuantity = 1000000}
        but got: Quantity {getQuantity = 0}

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_HW_WALLETS/HW_WALLETS_01x - Restoration from account public key preserves funds/"

Randomized with seed 109606617

@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 10, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 10, 2021

try

Build failed:

Completed 8 action(s).
--
  | /tmp/stack-8f9c70c9dfa1921e/test-ghc-env: openBinaryFile: resource busy (file is locked)
  |  
  | /tmp/stack-8f9c70c9dfa1921e/test-ghc-env: openBinaryFile: resource busy (file is locked)
  |  
  | /tmp/stack-8f9c70c9dfa1921e/test-ghc-env: openBinaryFile: resource busy (file is locked)
  |  
  | /tmp/stack-8f9c70c9dfa1921e/test-ghc-env: openBinaryFile: resource busy (file is locked)
  |  
  | /tmp/stack-8f9c70c9dfa1921e/test-ghc-env: openBinaryFile: resource busy (file is locked)
  | error: Command exited with code 1!
  | Continuing...

Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

So the reason for this change is that between checking one wallet condition and the next, the chain rolls back and invalidates the first condition, such that the second condition fails?

Before making a solution, do we have logging which proves conclusively that this is the case?

And what about other ways to solve this? We need some methods to make our tests less fragile - otherwise this flakiness will keep on happening.

My two ideas so far are:

  1. Provide API endpoints to pause and resume syncing for a wallet - preventing rollback while assertions are being checked.
  2. Provide a way for assertions to be contingent on a certain TxId having status=InLedger. If an assertion fails, and the Tx has gone back to Pending, then restart the eventually block from the beginning.

Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

Why not

@Anviking
Copy link
Member Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 11, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 08f4889 into master Jun 11, 2021
@iohk-bors iohk-bors bot deleted the anviking/ADP-970/waitForTxImmutability branch June 11, 2021 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants