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

Add a 'RunOnRepl' module #4358

Merged
merged 3 commits into from
Feb 20, 2023
Merged

Conversation

dnadales
Copy link
Member

... which allows to (re-)test a counterexample found by the ChainDB.StateMachine tests by copying and pasting it in the GHC repl.

Part of #4183

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • The documentation has been properly updated
    • New tests are added if needed and existing tests are updated
    • Any changes affecting Consensus packages must have an entry in the appropriate changelog.d directory created using scriv. If in doubt, see the Consensus release process.
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@dnadales dnadales requested a review from nfrisby as a code owner February 10, 2023 10:04
@dnadales dnadales requested review from a user, bartfrenk and jorisdral and removed request for nfrisby and jorisdral February 10, 2023 10:05
-- * Running the counterexamples
quickCheckeCmdsLockStep
-- * Patterns needed to disambiguate the 'At' and 'Command' symbols printed
-- * by the 'ChainDB.StateMachine' tests.
Copy link

Choose a reason for hiding this comment

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

typo: Remove leading * as I don't think you want another list item here

Copy link
Member Author

Choose a reason for hiding this comment

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

Ups, M-q 😁 Thanks.

@@ -228,6 +228,7 @@ test-suite test-storage
Test.Ouroboros.Storage.ChainDB.Model.Test
Test.Ouroboros.Storage.ChainDB.Paths
Test.Ouroboros.Storage.ChainDB.StateMachine
Test.Ouroboros.Storage.ChainDB.StateMachine.RunOnRepl
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Why not just Test.Ouroboros.Storage.ChainDB.StateMachine.REPL

Copy link
Member Author

Choose a reason for hiding this comment

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

My thought: it's not a REPL, but rather imports to allow the state machine to be run on the REPL. What do you think?

pattern Command cmd rsp xs =
StateMachine.Types.Command (StateMachine.At cmd) (StateMachine.At rsp) xs

quickCheckeCmdsLockStep ::
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo?

Suggested change
quickCheckeCmdsLockStep ::
quickCheckCmdsLockStep ::

-> SmallChunkInfo
-> Commands (StateMachine.At Cmd TestBlock IO) (StateMachine.At Resp TestBlock IO)
-> IO ()
quickCheckeCmdsLockStep maxClockSkew chunkInfo cmds =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
quickCheckeCmdsLockStep maxClockSkew chunkInfo cmds =
quickCheckCmdsLockStep maxClockSkew chunkInfo cmds =

@dnadales dnadales force-pushed the dnadales/4183-module-to-run-cmds-in-repl branch 2 times, most recently from 94fbac3 to 9654bfe Compare February 15, 2023 17:02
@dnadales dnadales force-pushed the dnadales/4183-module-to-run-cmds-in-repl branch from 9654bfe to b4f9fbf Compare February 16, 2023 10:12
Copy link
Contributor

@jasagredo jasagredo left a comment

Choose a reason for hiding this comment

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

Perhaps we could name this module .Utils.RunOnRepl to convey that it is not a test in itself but utilities for the tests?

--
-- > quickCheckCmdsLockStep someClockSkew someChunkInfo counterexample
--
-- Where 'someClockSkew' and 'someChunkInfo' are the ones given by a the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-- Where 'someClockSkew' and 'someChunkInfo' are the ones given by a the
-- Where 'someClockSkew' and 'someChunkInfo' are the ones given by the

@dnadales
Copy link
Member Author

Perhaps we could name this module .Utils.RunOnRepl to convey that it is not a test in itself but utilities for the tests?

Makes sense. I can rename the module.

... which allows to (re-)test a counterexample found by the
`ChainDB.StateMachine` tests by copying and pasting it in the GHC repl.
@dnadales dnadales force-pushed the dnadales/4183-module-to-run-cmds-in-repl branch from b4f9fbf to f4909bd Compare February 16, 2023 17:13
@dnadales
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 16, 2023
4358: Add a 'RunOnRepl' module r=dnadales a=dnadales

... which allows to (re-)test a counterexample found by the `ChainDB.StateMachine` tests by copying and pasting it in the GHC repl.

Part of #4183 



Co-authored-by: Damian Nadales <damian.only@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 16, 2023

Build failed:

@dnadales
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 17, 2023
4358: Add a 'RunOnRepl' module r=dnadales a=dnadales

... which allows to (re-)test a counterexample found by the `ChainDB.StateMachine` tests by copying and pasting it in the GHC repl.

Part of #4183 



Co-authored-by: Damian Nadales <damian.only@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 17, 2023

Build failed:

@dnadales
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 17, 2023
4358: Add a 'RunOnRepl' module r=dnadales a=dnadales

... which allows to (re-)test a counterexample found by the `ChainDB.StateMachine` tests by copying and pasting it in the GHC repl.

Part of #4183 



Co-authored-by: Damian Nadales <damian.only@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 17, 2023

Build failed:

@dnadales
Copy link
Member Author

bors retry

iohk-bors bot added a commit that referenced this pull request Feb 17, 2023
4358: Add a 'RunOnRepl' module r=dnadales a=dnadales

... which allows to (re-)test a counterexample found by the `ChainDB.StateMachine` tests by copying and pasting it in the GHC repl.

Part of #4183 



Co-authored-by: Damian Nadales <damian.only@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 17, 2023

Build failed:

... to distinguish this module from a test suite.
@dnadales dnadales force-pushed the dnadales/4183-module-to-run-cmds-in-repl branch from f4909bd to f2802c3 Compare February 20, 2023 10:01
@dnadales
Copy link
Member Author

bors r+

@iohk-bors iohk-bors bot merged commit 18ea4c3 into master Feb 20, 2023
@iohk-bors iohk-bors bot deleted the dnadales/4183-module-to-run-cmds-in-repl branch February 20, 2023 12:18
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.

3 participants