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

Execute before and after for each spec #2299

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

barnesjohnraymond
Copy link

This pull request aims to fix the issue described in #2270. The before and after actions should always be executed for each spec. Setting these values to false actually only makes sense for testing purposes.

Signed-off-by: John Barnes barnesjohnraymond@gmail.com

Signed-off-by: John Barnes <barnesjohnraymond@gmail.com>
Signed-off-by: John Barnes <barnesjohnraymond@gmail.com>
Signed-off-by: John Barnes <barnesjohnraymond@gmail.com>
@barnesjohnraymond
Copy link
Author

@zabil
Could you please have a look at this pull request and, if it looks ok for you, approve the workflows to be run?

@barnesjohnraymond
Copy link
Author

@zabil @sriv @johnboyes
I cannot find out who else is a maintainer of this project but could someone of the maintainers please have a look at this pull request and approve the open workflows? Thank you!

@sriv
Copy link
Member

sriv commented Oct 31, 2022

@barnesjohnraymond - thanks for the ping and apologies for not getting back earlier.

I looked at the changeset, and I can see that you've removed some state checks. This code was written a while back as part of a7d368b and 716adb3.

Looking at the commit history, I feel that removing the state checks will break the table driven execution for the spec (note that for table driven execution, it is expected that the before/after are run once).

I am going to try and spend some time to understand #2270 better, let's discuss this further there. I'll keep this PR open for now.

@chadlwilson chadlwilson marked this pull request as draft October 27, 2023 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants