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

Some state machine test fixes and refactorings #556

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jorisdral
Copy link
Collaborator

No description provided.

@jorisdral jorisdral marked this pull request as draft January 29, 2025 18:15
@jorisdral jorisdral self-assigned this Jan 29, 2025
@jorisdral jorisdral marked this pull request as ready for review January 30, 2025 09:24
Also, rename `ErrFsError` to `ErrDiskFault`
Previously, if `checkForgottenRefs` threw an error, no counterexample output
would be produced by the test. Now that is a property, counterexamples are
properly printed again.
Changes include:

* Add a new `ErrOther` model error.
* Define a separate error handler for each specific type of SUT exception.
* Create `realErrorHandlers`, the set of error handlers used on the SUT.
* Define a new `catchAllErrorHandler`, which matches on `SomeException` and maps
  it to `ErrOther`.

The main intent of this refactoring is to map *all* SUT exceptions to a model
`Err`. If a SUT exception is not explicitly caught by specialised error handlers
such as `lsmTreeErrorHandler`, then `catchAllErrorHandler` will make sure to
catch the exception and map it to the catch-all `ErrOther` value.

As a result, we get proper counterexample output, which would not happen if we
were to get an uncaught exception in the test.
@@ -351,6 +340,14 @@ propLockstep_RealImpl_MockFS_IO tr =
)
tagFinalState'

-- We can not use @bracket@ inside @PropertyM@, so @acuire_RealImpl_MockFS@ and
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-- We can not use @bracket@ inside @PropertyM@, so @acuire_RealImpl_MockFS@ and
-- We can not use @bracket@ inside @PropertyM@, so @acquire_RealImpl_MockFS@ and

@@ -351,6 +340,14 @@ propLockstep_RealImpl_MockFS_IO tr =
)
tagFinalState'

-- We can not use @bracket@ inside @PropertyM@, so @acuire_RealImpl_MockFS@ and
-- @release_RealImpl_MockFS@ are not run in a maksed state and it is not
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-- @release_RealImpl_MockFS@ are not run in a maksed state and it is not
-- @release_RealImpl_MockFS@ are not run in a masked state and it is not

hasBlockIO <- ioHasBlockIO hasFS defaultIOCtxParams
pure (tempDir, hasFS, hasBlockIO)
-- | When combined with other handlers, 'catchAllErrorHandler' has to go last
-- because it matches on 'SomeException', an no other handlers are run after
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-- because it matches on 'SomeException', an no other handlers are run after
-- because it matches on 'SomeException', and no other handlers are run after

Comment on lines +284 to +298
-- Approximate equality for errors.
--
-- This approximate equality satisfies the __Reflexivity__, __Symmetry__, and
-- __Negation__ laws for the 'Eq' class, but not __Transitivity__ and
-- __Substitutivity__.
--
-- These laws should be sufficient for the purpose of comparing an error
-- returned from the SUT to an error returned from the model.
--
-- Where @'Eq' 'Err'@ deviates from a typical instance is that 'ErrDiskFault' is
-- equal to all other errors. This is important for comparing SUT exceptions to
-- model exceptions. The model only knows /something/ went wrong, but the SUT
-- will probably have a very specific error that it throws. In such cases where
-- the SUT error is more specific then the model error, the errors are
-- considered equal.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This unlawful instance seems quite hacky. Is using a plain function difficult because quickcheck-lockstep asks for an Eq instance?

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