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.

Sign up for GitHub

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

Jump to bottom

Attempt to fix "Gave up!" problem in prop_balanceTransactionUnresolvedInputs #3212

Merged
merged 3 commits into from
Apr 1, 2022
Merged

Attempt to fix "Gave up!" problem in prop_balanceTransactionUnresolvedInputs #3212

merged 3 commits into from
Apr 1, 2022

Conversation

Anviking
Copy link
Member

  • Simply removing ==> seems to work fine, and I believe makes it more likely to
    pass.
  • Remove withMaxSuccess because it seems to be doing nothing in
    conjunction with checkCoverage.

Comments

  • I'm not entirely sure of the interplay between the various ==>, checkCoverage, etc, not certain that this for sure is a fix, however, it can't hurt to try.
$ cabal run cardano-wallet:test:unit -- --match "unresolved inputs" --seed 1053383965                                                                                                                                                    
Cardano.Wallet.Shelley.Transaction
  balanceTransaction
    when passed unresolved inputs
      may fail (1697ms)
        +++ OK, passed 100 tests:
        10% unknown txins

        87% other error
         3% nothing dropped

Finished in 1.6980 seconds, used 1.7075 seconds of CPU time
1 example, 0 failures

Issue Number

ADP-1590

@Anviking Anviking self-assigned this Mar 31, 2022
@Anviking
Fix "Gave up!" problem in property
581bc08 
- Simply removing `==>` seems to work fine, and make it more likely to
  pass.
- Remove `withMaxSuccess` because it seems to be doing nothing in
  conjunction with `checkCoverage`.
@Anviking Anviking force-pushed the anviking/ADP-1590/fix-unresolved-input-property branch from 7c59a5b to 581bc08 Compare March 31, 2022 11:41
jonathanknowles
jonathanknowles approved these changes Apr 1, 2022
View reviewed changes
Copy link
Contributor

@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.

Looks okay to me!

I only have one suggestion: it looks like one of the comments has a typo: I've provided a suggested fix. (I suspect it's not correct, but feel free to point out if I'm mistaken!)

lib/shelley/test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs Outdated Show resolved Hide resolved
@Anviking @jonathanknowles
Update lib/shelley/test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs
b46ee18 
Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
@Anviking
Copy link
Member Author

Anviking commented Apr 1, 2022

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 1, 2022
@iohk-bors @Anviking
Merge #3212
2492c03 
3212: Attempt to fix "Gave up!" problem in prop_balanceTransactionUnresolvedInputs r=Anviking a=Anviking

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

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- Simply removing `==>` seems to work fine, and I believe makes it more likely to
  pass.
- Remove `withMaxSuccess` because it seems to be doing nothing in
  conjunction with `checkCoverage`.

### Comments

- I'm not entirely sure of the interplay between the various `==>`, `checkCoverage`, etc, not certain that this _for sure_ is a fix, however, it can't hurt to try.

```
$ cabal run cardano-wallet:test:unit -- --match "unresolved inputs" --seed 1053383965                                                                                                                                                    
Cardano.Wallet.Shelley.Transaction
  balanceTransaction
    when passed unresolved inputs
      may fail (1697ms)
        +++ OK, passed 100 tests:
        10% unknown txins

        87% other error
         3% nothing dropped

Finished in 1.6980 seconds, used 1.7075 seconds of CPU time
1 example, 0 failures
```

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

### Issue Number

ADP-1590

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


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

iohk-bors bot commented Apr 1, 2022 •
edited by Anviking
Loading

Build failed:

  test/unit/Cardano/Wallet/Api/Server/TlsSpec.hs:96: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                 = 42589
         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 1699334848

@Anviking
Copy link
Member Author

Anviking commented Apr 1, 2022

bors r+

iohk-bors bot added a commit that referenced this pull request Apr 1, 2022
@iohk-bors @Anviking
Merge #3212
9cd4d39 
3212: Attempt to fix "Gave up!" problem in prop_balanceTransactionUnresolvedInputs r=Anviking a=Anviking

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

Before you submit, don't forget to:

* Make sure the GitHub PR fields are correct:
   ✓ Set a good Title for your PR.
   ✓ Assign yourself to the PR.
   ✓ Assign one or more reviewer(s).
   ✓ Link to a Jira issue, and/or other GitHub issues or PRs.
   ✓ In the PR description delete any empty sections
     and all text commented in <!--, so that this text does not appear
     in merge commit messages.

* Don't waste reviewers' time:
   ✓ If it's a draft, select the Create Draft PR option.
   ✓ Self-review your changes to make sure nothing unexpected slipped through.

* Try to make your intent clear:
   ✓ Write a good Description that explains what this PR is meant to do.
   ✓ Jira will detect and link to this PR once created, but you can also
     link this PR in the description of the corresponding Jira ticket.
   ✓ Highlight what Testing you have done.
   ✓ Acknowledge any changes required to the Documentation.
-->


- Simply removing `==>` seems to work fine, and I believe makes it more likely to
  pass.
- Remove `withMaxSuccess` because it seems to be doing nothing in
  conjunction with `checkCoverage`.

### Comments

- I'm not entirely sure of the interplay between the various `==>`, `checkCoverage`, etc, not certain that this _for sure_ is a fix, however, it can't hurt to try.

```
$ cabal run cardano-wallet:test:unit -- --match "unresolved inputs" --seed 1053383965                                                                                                                                                    
Cardano.Wallet.Shelley.Transaction
  balanceTransaction
    when passed unresolved inputs
      may fail (1697ms)
        +++ OK, passed 100 tests:
        10% unknown txins

        87% other error
         3% nothing dropped

Finished in 1.6980 seconds, used 1.7075 seconds of CPU time
1 example, 0 failures
```

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

### Issue Number

ADP-1590

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


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

iohk-bors bot commented Apr 1, 2022

Build failed:

@Anviking
Regenerate PKI test data for TlsSpec
497d15c 
It seems they have expired again...
@Anviking
Copy link
Member Author

Anviking commented Apr 1, 2022

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 1, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 8a50869 into master Apr 1, 2022
@iohk-bors iohk-bors bot deleted the anviking/ADP-1590/fix-unresolved-input-property branch April 1, 2022 13:43
WilliamKingNoel-Bot pushed a commit that referenced this pull request Apr 1, 2022
docs: Merge #3212
7a7afb2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanknowles jonathanknowles jonathanknowles approved these changes

Assignees

@Anviking Anviking

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Anviking @jonathanknowles