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

Decrease coverage requirement in UTxOIndexSpec #2574

Merged
merged 1 commit into from
Apr 2, 2021

Conversation

Anviking
Copy link
Collaborator

@Anviking Anviking commented Mar 23, 2021

Issue Number

#2575

Overview

  • Decrease all coverage 80 to coverage 70 inside UTxOIndexSpec

Comments

@Anviking Anviking added Test failure A flaky test or nightly CI failure and removed Test failure A flaky test or nightly CI failure labels Mar 23, 2021
@Anviking Anviking self-assigned this Mar 24, 2021
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.

Do you happen to know why cover 80 was chosen before, and why it's now too much?

@Anviking
Copy link
Collaborator Author

Anviking commented Mar 29, 2021

I simply saw the #2575 failure, and thought it was an innocent slightly-to-strict cover-limit from when the tests were introduced. cc @jonathanknowles ?

@jonathanknowles
Copy link
Member

Do you happen to know why cover 80 was chosen before, and why it's now too much?

When we originally set these coverage requirements, the average real coverage was consistently higher. (I recall running these tests hundreds of times to make sure.)

However, if the underlying generators have recently been changed, then the average coverage could have dropped, causing the tests to sporadically fail.

Nevertheless, 70 should still be more than adequate here.

Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

Hi @Anviking!

This looks fine to me.

See #2574 (comment)

@Anviking
Copy link
Collaborator Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 30, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 30, 2021

Build failed:

@Anviking
Copy link
Collaborator Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 30, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 30, 2021

Build failed:

@Anviking
Copy link
Collaborator Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 30, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 30, 2021

Build failed:

Failures:

  test/unit/Cardano/Wallet/Api/Server/TlsSpec.hs:79:5: 
  1) Cardano.Wallet.Api.Server.Tls, TLS Client Authentication, Respond to authenticated client if TLS is enabled
       uncaught exception: HttpException
       HttpExceptionRequest Request {
         host                 = "127.0.0.1"
         port                 = 38153
         secure               = True
         requestHeaders       = []
         path                 = "/"
         queryString          = ""
         method               = "GET"
         proxy                = Nothing
         rawBody              = False
         redirectCount        = 10
         responseTimeout      = ResponseTimeoutDefault
         requestVersion       = HTTP/1.1
       }
        (InternalException (Terminated True "received fatal error: CertificateUnknown" (Error_Protocol ("remote side fatal error",True,CertificateUnknown))))

  To rerun use: --match "/Cardano.Wallet.Api.Server.Tls/TLS Client Authentication/Respond to authenticated client if TLS is enabled/"

Randomized with seed 392927842

@Anviking
Copy link
Collaborator Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 30, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 30, 2021

Build failed:

Failures:

  test/unit/Cardano/Wallet/Api/Server/TlsSpec.hs:79:5: 
  1) Cardano.Wallet.Api.Server.Tls, TLS Client Authentication, Respond to authenticated client if TLS is enabled
       uncaught exception: HttpException
       HttpExceptionRequest Request {
         host                 = "127.0.0.1"
         port                 = 42167
         secure               = True
         requestHeaders       = []
         path                 = "/"
         queryString          = ""
         method               = "GET"
         proxy                = Nothing
         rawBody              = False
         redirectCount        = 10
         responseTimeout      = ResponseTimeoutDefault
         requestVersion       = HTTP/1.1
       }
        (InternalException (Terminated True "received fatal error: CertificateUnknown" (Error_Protocol ("remote side fatal error",True,CertificateUnknown))))

  To rerun use: --match "/Cardano.Wallet.Api.Server.Tls/TLS Client Authentication/Respond to authenticated client if TLS is enabled/"

Randomized with seed 1225658022

@Anviking
Copy link
Collaborator Author

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 30, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 30, 2021

Build failed:

@Anviking
Copy link
Collaborator Author

Anviking commented Apr 1, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 1, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 1, 2021

Build failed:

cardano-wallet                 >
--
  | cardano-wallet                 >   src/Test/Integration/Framework/DSL.hs:2017:7:
  | cardano-wallet                 >   1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_JOIN_01rewards - Can join a pool, earn rewards and collect them
  | cardano-wallet                 >        expected a successful response but got an error: DecodeFailure "{\"code\":\"created_invalid_transaction\",\"message\":\"That's embarrassing. It looks like I've created an invalid transaction that could not be parsed by the node. Here's an error message that may help with debugging: HardForkApplyTxErrFromEra S (S (S (Z (WrapApplyTxErr {unwrapApplyTxErr = ApplyTxError [LedgerFailure (DelegsFailure (WithdrawalsNotInRewardsDELEGS (fromList [(RewardAcnt {getRwdNetwork = Mainnet, getRwdCred = KeyHashObj (KeyHash \\\"8fb209fce870f9ffe39ebe853fc838f8e289ace1e7eaa230378827e6\\\")},Coin 1534631)])))]}))))\"}"
  | cardano-wallet                 >        While verifying (Status {statusCode = 500, statusMessage = "Internal Server Error"},Left (DecodeFailure "{\"code\":\"created_invalid_transaction\",\"message\":\"That's embarrassing. It looks like I've created an invalid transaction that could not be parsed by the node. Here's an error message that may help with debugging: HardForkApplyTxErrFromEra S (S (S (Z (WrapApplyTxErr {unwrapApplyTxErr = ApplyTxError [LedgerFailure (DelegsFailure (WithdrawalsNotInRewardsDELEGS (fromList [(RewardAcnt {getRwdNetwork = Mainnet, getRwdCred = KeyHashObj (KeyHash \\\"8fb209fce870f9ffe39ebe853fc838f8e289ace1e7eaa230378827e6\\\")},Coin 1534631)])))]}))))\"}"))
  | cardano-wallet                 >
  | cardano-wallet                 >   To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_JOIN_01rewards - Can join a pool, earn rewards and collect them/"
  | cardano-wallet                 >

#2467

@Anviking
Copy link
Collaborator Author

Anviking commented Apr 1, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 1, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 1, 2021

Build failed:

Has been failing twice recently. Latest failure was

Failures:

 test/unit/Cardano/Wallet/Primitive/Types/UTxOIndexSpec.hs:146:9:
 1) Cardano.Wallet.Primitive.Types.UTxOIndex, Indexed UTxO set properties, Index Selection, prop_selectRandom_one_withAsset
 Insufficient coverage (after 800 tests):
 89.5% index has more than one asset
 71.1% index has the specified asset
 71.1% selected an entry

 Only 71.1% index has the specified asset, but expected 80.0%
 Only 71.1% selected an entry, but expected 80.0%

 To rerun use: --match "/Cardano.Wallet.Primitive.Types.UTxOIndex/Indexed UTxO set properties/Index Selection/prop_selectRandom_one_withAsset/"

 test/unit/Cardano/Wallet/Primitive/Types/UTxOIndexSpec.hs:148:9:
 2) Cardano.Wallet.Primitive.Types.UTxOIndex, Indexed UTxO set properties, Index Selection, prop_selectRandom_one_withAssetOnly
 Insufficient coverage (after 800 tests):
 89.5% index has more than one asset
 71.1% index has the specified asset
 22.5% selected an entry

 Only 71.1% index has the specified asset, but expected 80.0%

 To rerun use: --match "/Cardano.Wallet.Primitive.Types.UTxOIndex/Indexed UTxO set properties/Index Selection/prop_selectRandom_one_withAssetOnly/"

 test/unit/Cardano/Wallet/Primitive/Types/UTxOIndexSpec.hs:154:9:
 3) Cardano.Wallet.Primitive.Types.UTxOIndex, Indexed UTxO set properties, Index Selection, prop_selectRandom_all_withAsset
 Insufficient coverage (after 800 tests):
 89.5% index has more than one asset
 71.1% index has the specified asset
 71.1% selected at least one entry

 Only 71.1% index has the specified asset, but expected 80.0%
 Only 71.1% selected at least one entry, but expected 80.0%

 To rerun use: --match "/Cardano.Wallet.Primitive.Types.UTxOIndex/Indexed UTxO set properties/Index Selection/prop_selectRandom_all_withAsset/"

 test/unit/Cardano/Wallet/Primitive/Types/UTxOIndexSpec.hs:156:9:
 4) Cardano.Wallet.Primitive.Types.UTxOIndex, Indexed UTxO set properties, Index Selection, prop_selectRandom_all_withAssetOnly
 Insufficient coverage (after 800 tests):
 89.5% index has more than one asset
 71.1% index has the specified asset
 22.5% selected at least one entry

 Only 71.1% index has the specified asset, but expected 80.0%

 To rerun use: --match "/Cardano.Wallet.Primitive.Types.UTxOIndex/Indexed UTxO set properties/Index Selection/prop_selectRandom_all_withAssetOnly/"
@Anviking
Copy link
Collaborator Author

Anviking commented Apr 1, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 1, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 1, 2021

Build failed:

@Anviking
Copy link
Collaborator Author

Anviking commented Apr 1, 2021

bors r+
pls

iohk-bors bot added a commit that referenced this pull request Apr 1, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 1, 2021

Build failed:

#duplicate (cached failure)

@Anviking
Copy link
Collaborator Author

Anviking commented Apr 2, 2021

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 2, 2021
2574: Decrease coverage requirement in UTxOIndexSpec r=Anviking a=Anviking


# Issue Number

#2575

<!-- Put here a reference to the issue that this PR relates to and which requirements it tackles. Jira issues of the form ADP- will be auto-linked. -->


# Overview

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

- [x] Decrease all `coverage 80` to `coverage 70` inside UTxOIndexSpec


# Comments


<!--
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)
 ✓ Jira will detect and link to this PR once created, but you can also link this PR in the description of the corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
 ✓ Finally, in the PR description delete any empty sections and all text commented in <!--, so that this text does not appear in merge commit messages.
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 2, 2021

Build failed:

cardano-wallet                 >   src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:152:26:
--
  | cardano-wallet                 >   1) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_01x - Restoration from account public key preserves funds
  | cardano-wallet                 >        While verifying ApiWallet {id = ApiT {getApiT = WalletId {getWalletId = d7218c30c579c9e96e4a7ce311e96f299474d782}}, addressPoolGap = ApiT {getApiT = AddressPoolGap {getAddressPoolGap = 20}}, balance = ApiWalletBalance {available = Quantity {getQuantity = 0}, total = Quantity {getQuantity = 0}, 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 = "Wallet from pub key"}}, passphrase = Nothing, state = ApiT {getApiT = Syncing (Quantity {getQuantity = Percentage {getPercentage = 3777 % 5000}})}, tip = ApiBlockReference {absoluteSlotNumber = ApiT {getApiT = SlotNo 5942}, slotId = ApiSlotId {epochNumber = ApiT {getApiT = EpochNo {unEpochNo = 118}}, slotNumber = ApiT {getApiT = SlotInEpoch {unSlotInEpoch = 42}}}, time = 2021-04-02 19:21:03.4 UTC, block = ApiBlockInfo {height = Quantity {getQuantity = 3000}}}}
  | cardano-wallet                 >        Waited longer than 90s to resolve action: "Wallet balance is as expected on wallet from pubKey".


#2428

@Anviking
Copy link
Collaborator Author

Anviking commented Apr 2, 2021

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 2, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 7acc8ac into master Apr 2, 2021
@iohk-bors iohk-bors bot deleted the anviking/lower-coverage branch April 2, 2021 20:40
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.

None yet

3 participants