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

Short circuit on failed assertion #21

Closed
caioaao opened this issue Feb 2, 2019 · 6 comments
Closed

Short circuit on failed assertion #21

caioaao opened this issue Feb 2, 2019 · 6 comments

Comments

@caioaao
Copy link
Contributor

caioaao commented Feb 2, 2019

I would expect a failed assertion (i.e.: failed verify or match?) would short circuit a flow. This is not the case and as of now we don't have an easy way of asserting that it happened when running a flow.

caioaao added a commit to caioaao/state-flow that referenced this issue Feb 2, 2019
Fixes nubank#21

The `assert` macro receives a body and, if this body doesn't return a truthy
value, it will throw an exception with the details to short circuit the flow.
caioaao added a commit to caioaao/state-flow that referenced this issue Feb 4, 2019
Fixes nubank#21

The `assert` macro receives a body and, if this body doesn't return a truthy
value, it will throw an exception with the details to short circuit the flow.
@caioaao caioaao mentioned this issue Feb 4, 2019
@caioaao
Copy link
Contributor Author

caioaao commented Feb 4, 2019

A way to check for failed runs that I think is interesting is used in kaocha through clojure test's report counters: https://github.com/lambdaisland/kaocha/blob/master/src/kaocha/type.clj
it's not really clean, but might be a good way of decoupling the state-flow framework from the assertions.

@philomates
Copy link
Contributor

One thing to keep in mind:
I believe the way that kaocha uses clojure.test counters, as well as selvage, which copied kaocha, is that it has issues with test runners that run tests in parallel. I think this is why bat-test doesn't play well with selvage's defflow.
I want to try to fix selvage to run tests in parallel at some point so I'll try to do it in a way that can be used here as well

@sovelten
Copy link
Collaborator

sovelten commented Aug 6, 2019

I don't see a strong reason to short circuit. I actually see reasons for not short circuiting. Supposing you do an action that has several consequences, than you make several assertions for each of the consequences. These assertions are independent and it is meaningful to know which of them passed or failed. If the flow didn't fail catastrophically (with an exception) it probably should continue. The spice must flow.

But I'm all open for having this behavior configurable. And also decoupling state-flow from test assertions

@caioaao
Copy link
Contributor Author

caioaao commented Aug 6, 2019

I agree with the multiple assertion, but not steps. If an assertion fails between two steps, the step didn't work as expected and imo it doesn't make sense to keep running the next steps (assuming a flow is exactly for actions that depend on the effect of previous actions)

@dchelimsky
Copy link
Contributor

If an assertion fails between two steps, the step didn't work as expected and imo it doesn't make sense to keep running the next steps

To the runner, flows, steps, assertions, etc, are all the same thing, so there is no way to differentiate between a step and an assertion. Also, since you can arbitrarily nest flows inside assertions inside flows, there is really no way to identify when "an assertion fails between two steps"

I think the simplest thing is to either short circuit on error or not (configurable - default TBD), and runtime exceptions should be treated the same as failed assertions.

@philomates
Copy link
Contributor

Hey, so the run* function was extended to take a fail-fast? option to achieve this. This has been released in 5.6.0 via #138

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 a pull request may close this issue.

4 participants