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

Parallelise integration tests using ResourceT #2191

Merged
merged 28 commits into from
Nov 4, 2020

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Sep 29, 2020

Issue Number

ADP-466 — both ADP-529 and ADP-528

Overview

  • Use runResourceT in each test scenario to ensure only the wallet created in it are deleted.
  • Re-write all listWallets expectations to check that the expected wallets are a subset of the actual wallets, instead of checking for equality.
  • Enable -j 8 in buildkite

Comments

  • Buildkite now takes ≈35 min instead of 50 min
  • On my machine stack test cardano-wallet:integration --ta '-j 8' takes 380s instead of ~1800s.
  • Haven't enabled parallel tests in hydra yet because of errors
  • I now pushed anviking/ADP-466/parallel (parallel everything) directly here.

@Anviking Anviking self-assigned this Sep 29, 2020
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Sep 29, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 29, 2020

try

Build failed:

@Anviking Anviking force-pushed the anviking/replace-tearDown branch from 6c1613f to ddab4d3 Compare September 29, 2020 16:26
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Sep 29, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 29, 2020

try

Build failed:

@rvl
Copy link
Contributor

rvl commented Sep 30, 2020

This is an excellent idea @Anviking - a general way to clean up resources allocated during test execution.

  1. Just wondering, any reason for choosing the resourcet library ahead of other possible options such as managed?

  2. Is there a general strategy for handling errors in the resource destroy function? If a test case throws an exception, and the destructor also throws an exception, which one is reported to the user?

  3. Looking at this type which is repeated many times.

    Context t -> ResourceT m a

    Since you have introduced a monad transformer, what about stacking a reader on top for the Context?

@Anviking
Copy link
Member Author

Just wondering, any reason for choosing the resourcet library ahead of other possible options such as managed?

I went for resourcet because I think I heard Edsko mention it in Albi, and I didn't know about managed. Looking now, it seems to me like resourcet is more powerful (allows unprotect, some stuff for sharing resources across threads), which might come in handy one day.

@Anviking Anviking force-pushed the anviking/replace-tearDown branch from 6dce2e8 to f00890c Compare October 1, 2020 13:30
@Anviking
Copy link
Member Author

Anviking commented Oct 1, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Oct 1, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 1, 2020

try

Build failed:

 To rerun use: --match "/CLI Specifications/SHELLEY_CLI_WALLETS/WALLETS_DELETE_01, WALLETS_LIST_02 - Can delete wallet/"

  src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:330:13:
  10) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_05 - Wallet from pubKey is available, The same account and mnemonic wallet can live side-by-side
       expected: 2
        but got: 39

buildkite:

src/Test/Integration/Scenario/CLI/Shelley/Wallets.hs:317:13:
--
  | 1) CLI Specifications, SHELLEY_CLI_WALLETS, WALLETS_CREATE_05 - Can't create wallet with wrong size of mnemonic, 9
  | expected: "[]\n"
  | but got: "[\n    {\n        \"passphrase\": {\n            \"last_updated_at\": \"2020-10-01T19:25:32.709710148Z\"\n        },\n


@Anviking
Copy link
Member Author

Anviking commented Oct 1, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Oct 1, 2020
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Oct 1, 2020

try

Build failed:

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_WALLETS/WALLETS_DELETE_01, WALLETS_LIST_02 - Can delete wallet/"

  src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:330:13:
  10) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_05 - Wallet from pubKey is available, The same account and mnemonic wallet can live side-by-side
       expected: 2
        but got: 37

@Anviking
Copy link
Member Author

Anviking commented Oct 2, 2020

bors try

iohk-bors bot added a commit that referenced this pull request Oct 2, 2020
@Anviking
Copy link
Member Author

Anviking commented Oct 2, 2020

bors try-

@Anviking
Copy link
Member Author

Anviking commented Oct 8, 2020

Closing for now.

@Anviking Anviking closed this Oct 8, 2020
@Anviking Anviking reopened this Oct 19, 2020
@Anviking Anviking force-pushed the anviking/replace-tearDown branch 3 times, most recently from 1fde64f to 9a85557 Compare October 20, 2020 09:31
@Anviking

This comment has been minimized.

iohk-bors bot added a commit that referenced this pull request Oct 20, 2020
@Anviking Anviking force-pushed the anviking/replace-tearDown branch from 88626c0 to 68d9089 Compare October 20, 2020 11:24
@iohk-bors

This comment has been minimized.

Anviking and others added 20 commits November 4, 2020 14:46
To investigate failures like:

```
  src/Test/Integration/Scenario/API/Shelley/Transactions.hs:1037:18:
  2) API Specifications, SHELLEY_TRANSACTIONS, TRANS_EXTERNAL_02 - Multiple Outputs Transaction - Shelley witnesses
       Waited longer than 90s (more than 2 epochs) to resolve action: "wFaucet and wSrc balances are as expected".
       expected: Quantity {getQuantity = 999989842531}
        but got: Quantity {getQuantity = 999989842430}
```
such that it works in parallel. Removing the `walletId` check shouldn't
really matter.
To fix failures like:

```
 src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:185:13:
  1) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_03 - Cannot do operations requiring private key, Cannot send tx
       "Please enter your passphrase: **************\nI can't process this payment because there's not enough UTxO available in the wallet. The total UTxO sums up to 0 Lovelace, but I need 1000000 Lovelace (excluding fee amount) in order to proceed  with the payment.\n" does not contain "I couldn't find a root private key for the given wallet: 53a635ddc8caaee509f4bb991372bdbeeb4ebfde. However, this operation requires that I do have such a key.
```
Previously a `describe` with 3 `it`, each using the same wallet.

Race-conditions with the ResourceT cleanup... and simply parallel...
made this not work.

Let's simply convert them to a single test. We can use `counterexample`
to still show /which/ golden failed.
To investigate failures like:

```
  src/Test/Integration/Scenario/API/Shelley/Transactions.hs:1037:18:
  2) API Specifications, SHELLEY_TRANSACTIONS, TRANS_EXTERNAL_02 - Multiple Outputs Transaction - Shelley witnesses
       Waited longer than 90s (more than 2 epochs) to resolve action: "wFaucet and wSrc balances are as expected".
       expected: Quantity {getQuantity = 999989842531}
        but got: Quantity {getQuantity = 999989842430}
```
By mistake I first added the fix to TRANS_EXTERNAL_01 instead of
TRANS_EXTERNAL_02. But I'm not keeping it on both to be safe.
Thinking the following errors are just from an overloaded hydra...

 src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  1) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, mnemonic --size 15
       uncaught exception: IOException of type InvalidArgument
       fd:23: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/mnemonic --size 15/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  2) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, wallet list --port
       uncaught exception: IOException of type InvalidArgument
       fd:27: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/wallet list --port/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  3) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, wallet create
       uncaught exception: IOException of type InvalidArgument
       fd:28: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/wallet create/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  4) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, wallet
       uncaught exception: IOException of type InvalidArgument
       fd:23: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/wallet/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  5) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, create
       uncaught exception: IOException of type InvalidArgument
       fd:27: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/create/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  6) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, wallet crate name
       uncaught exception: IOException of type InvalidArgument
       fd:28: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/wallet crate name/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:102:34:
  7) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help on bad arg or param, network information --port
       uncaught exception: IOException of type InvalidArgument
       fd:24: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help on bad arg or param/network information --port/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:109:27:
  8) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help with, -h
       uncaught exception: IOException of type InvalidArgument
       fd:24: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help with/-h/"

  src/Test/Integration/Scenario/CLI/Miscellaneous.hs:109:27:
  9) No backend required, Miscellaneous CLI tests, COMMON_CLI_MISC, CLI_HELP - cardano-wallet shows help with, --help
       uncaught exception: IOException of type InvalidArgument
       fd:24: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/No backend required/Miscellaneous CLI tests/COMMON_CLI_MISC/CLI_HELP - cardano-wallet shows help with/--help/"

  src/Test/Integration/Scenario/CLI/Shelley/Addresses.hs:206:51:
  10) CLI Specifications, SHELLEY_CLI_ADDRESSES, ADDRESS_LIST_04 - False wallet ids, 41 chars hex
       uncaught exception: IOException of type InvalidArgument
       fd:514: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_ADDRESSES/ADDRESS_LIST_04 - False wallet ids/41 chars hex/"

  src/Test/Integration/Scenario/CLI/Shelley/Wallets.hs:306:40:
  11) CLI Specifications, SHELLEY_CLI_WALLETS, WALLETS_CREATE_05 - Can't create wallet with wrong size of mnemonic, 9
       uncaught exception: IOException of type InvalidArgument
       fd:594: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_WALLETS/WALLETS_CREATE_05 - Can't create wallet with wrong size of mnemonic/9/"

  src/Test/Integration/Scenario/CLI/Shelley/Wallets.hs:418:50:
  12) CLI Specifications, SHELLEY_CLI_WALLETS, WALLETS_CREATE_08 - --address-pool-gap values, arbitraty string -> fail
       uncaught exception: IOException of type InvalidArgument
       fd:633: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_WALLETS/WALLETS_CREATE_08 - --address-pool-gap values/arbitraty string -> fail/"

  src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:212:9:
  13) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_04 - Can manage HW wallet the same way as others, Can update name
       uncaught exception: IOException of type InvalidArgument
       fd:675: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_HW_WALLETS/HW_WALLETS_04 - Can manage HW wallet the same way as others/Can update name/"

  src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:379:46:
  14) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_06 - Test parameters, Address pool gap invalid, Pool gap: 9
       uncaught exception: IOException of type InvalidArgument
       fd:700: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/SHELLEY_CLI_HW_WALLETS/HW_WALLETS_06 - Test parameters/Address pool gap invalid/Pool gap: 9/"

  src/Test/Integration/Scenario/CLI/Port.hs:133:5:
  15) CLI Specifications, COMMON_CLI_PORTS, PORT_03 - Cannot omit --port when server uses random port (wallet create)
       uncaught exception: IOException of type InvalidArgument
       fd:643: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/COMMON_CLI_PORTS/PORT_03 - Cannot omit --port when server uses random port (wallet create)/"

  src/Test/Integration/Scenario/CLI/Port.hs:184:5:
  16) CLI Specifications, COMMON_CLI_PORTS, PORT_03 - Cannot omit --port when server uses random port (address list)
       uncaught exception: IOException of type InvalidArgument
       fd:694: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/COMMON_CLI_PORTS/PORT_03 - Cannot omit --port when server uses random port (address list)/"

  src/Test/Integration/Scenario/CLI/Port.hs:196:13:
  17) CLI Specifications, COMMON_CLI_PORTS, PORT_04 - Fail nicely when port is out-of-bounds, serve --port 1010
       uncaught exception: IOException of type InvalidArgument
       fd:642: hGetContents: invalid argument (invalid byte sequence)

  To rerun use: --match "/CLI Specifications/COMMON_CLI_PORTS/PORT_04 - Fail nicely when port is out-of-bounds/serve --port 1010/"
@Anviking Anviking force-pushed the anviking/replace-tearDown branch from 0625c46 to 1e4c82f Compare November 4, 2020 13:46
@Anviking
Copy link
Member Author

Anviking commented Nov 4, 2020

bors r+
(yolo)

iohk-bors bot added a commit that referenced this pull request Nov 4, 2020
2191: Parallelise integration tests using ResourceT r=Anviking a=Anviking

# Issue Number

ADP-466 (follow-up from ADP-419 )


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Use `runResourceT` in each test scenario to ensure only the wallet created in it are deleted.
- [x] Re-write all `listWallets` expectations to check that the expected wallets are a _subset_ of the actual wallets, instead of checking for equality.
- [x] Enable `-j 8` in buildkite
 


# Comments

- Buildkite now takes ≈35 min instead of 50 min
- On my machine `stack test cardano-wallet:integration --ta '-j 8'` takes 380s instead of ~1800s.
- Haven't enabled parallel tests in hydra yet because of errors
- I now pushed `anviking/ADP-466/parallel` (parallel _everything_) directly here.


<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: IOHK <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 4, 2020

Build failed:

Timeout:

    parallel puts replace values in _any_ order
      Checkpoint
        +++ OK, passed 100 tests (49% conflicting db entries).
      Wallet Metadata
        +++ OK, passed 100 tests (49% conflicting db entries).
      Tx History
building of '/nix/store/1g02d3ibj4sa37v4ppwj2hw7azqbw6df-cardano-wallet-core-test-unit-2020.11.3-check' timed out after 900 seconds of silence

@Anviking
Copy link
Member Author

Anviking commented Nov 4, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 4, 2020

Build succeeded:

@iohk-bors iohk-bors bot merged commit 0bf98c9 into master Nov 4, 2020
@iohk-bors iohk-bors bot deleted the anviking/replace-tearDown branch November 4, 2020 19:14
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