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

initial attempt at before and after hooks #3867

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dondod
Copy link

@dondod dondod commented Feb 10, 2024

This PR is an attempt to fix #2792. The before hook works, but I'm having trouble figuring out an approach for the after hook. I have a failing test for the after hook. In the current code, only the top-level after hook runs, so the assertion fails in that hook. It seems that I can either get all the after hooks to run, but run more than once (like afterEach) or I can get only one to run at all.

Any ideas on how to get each after hook to run exactly once, and starting with the deepest-nested hook?

dondod and others added 2 commits February 10, 2024 09:19
@Kantis
Copy link
Member

Kantis commented Feb 14, 2024

@dondod I adjusted your implementation slightly. Check my commit and see if it aligns with your interpretation of the issue.

@Kantis
Copy link
Member

Kantis commented Feb 14, 2024

@dondod api_check is failing, since the ABI doesn't match the dump. Please run ./gradlew apiDump and commit the updated .api file(s).

See https://github.com/kotlin/binary-compatibility-validator/ for more info

@dondod
Copy link
Author

dondod commented Feb 14, 2024

Thanks @Kantis - your changes do work. So simple too..

I'll run the apiDump task and commit.

@dondod
Copy link
Author

dondod commented Feb 15, 2024

Hi @Kantis - I don't see what is causing the test_linux jvmTest failure. Anything I need to change?

@Kantis
Copy link
Member

Kantis commented Feb 15, 2024

What I think is happening is that one of the tests is using a Kotest extension that has been built using an older version of Kotest. That version of Kotest's TestListener did not include the newly added BeforeListener, and thus it hasn't inherited the default implementation for that method.

Either we need to push this feature to 6.0 and do a breaking change and update all extensions (including third-party ones), or we must introduce a new interface and deprecate the old one. @sksamuel are there other ways to fix this?

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.

Nested "before" lifecycle hook
2 participants