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 optional initial transition #492

Conversation

dylan-hoefsloot
Copy link
Contributor

@dylan-hoefsloot dylan-hoefsloot commented Feb 22, 2023

This will allow the state machine to track the initial state for the machine, this is important for 4 reasons:

  • It allows us to keep a record of how long the state machine spent in the initial state
  • If the initial state configuration changes after the state machine transitions from the initial state we still have an accurate record of what the initial state was
  • If the initial state configuration changes before the state machine transitions from the initial state we still have an accurate record of what the initial state is
  • It allows us to utilise transition hooks for transitioning to the initial state

This will allow the state machine to track the initial state for the machine, this is important for 3 reasons:
- It allows us to keep a record of how long the state machine spent in the initial state
- If the initial state configuration changes after the state machine transitions from the initial state we still have an accurate record of what the initial state was
- If the initial state configuration changes before the state machine transitions from the initial state we still have an accurate record of what the initial state is
expect { check_missing_methods! }.to_not raise_exception(NotImplementedError)
expect { check_missing_methods! }.to_not raise_exception
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WARNING: Using `expect { }.not_to raise_error(SpecificErrorClass)` risks false positives, since literally any other error would cause the expectation to pass, including those raised by Ruby (e.g. NoMethodError, NameError and ArgumentError), meaning the code you are intending to test may not even get reached. Instead consider using `expect { }.not_to raise_error` or `expect { }.to raise_error(DifferentSpecificErrorClass)`.

Copy link
Contributor

@Tabby Tabby left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. Needs updating to current revision though, so that the CI checks run

spec/statesman/machine_spec.rb Outdated Show resolved Hide resolved
@dylan-hoefsloot
Copy link
Contributor Author

@Tabby Any chance we get this merged?

@stephenbinns stephenbinns merged commit 52e3a20 into gocardless:master Jan 5, 2024
78 checks passed
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