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

ARQ-2231 The JUnit 5 container does not work with manual mode tests #583

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

rhusar
Copy link
Collaborator

@rhusar rhusar commented Jul 3, 2024

Restoring the fix for https://issues.redhat.com/browse/ARQ-2231 which was inadvertently reverted along restoring the BeforeEachCallback, AfterEachCallback callbacks which turned out to be necessary in some cases. The reproducer attached to ARQ-2231 passes now.

@rhusar
Copy link
Collaborator Author

rhusar commented Jul 3, 2024

@jamezp any chance your tests in the other PR can be easily adapted to test this?

@jamezp
Copy link
Collaborator

jamezp commented Jul 3, 2024

@jamezp any chance your tests in the other PR can be easily adapted to test this?

It's definitely my plan to add manual mode tests. I'm not sure I'll be able to get to that until next week though.

@starksm64 starksm64 merged commit fc45617 into arquillian:master Jul 3, 2024
3 checks passed
@rhusar rhusar deleted the ARQ-2231-restore branch July 8, 2024 13:10
@rhusar
Copy link
Collaborator Author

rhusar commented Jul 8, 2024

@jamezp @starksm64 So given all the changes that went into the JUnit 5 extension we are effectively functionally same as the changes proposed by @petrberan in #544 so kudos to him for seeing and doing this first!

What I we are still missing is: (1) I am still not sure why we need duplicate before and after notifications (if this was explained somewhere please re-share this and ideally add a note to this class as to why for future reference) and (2) we still need to have this covered by tests - we have a test attached to the Jira which needs to be transformed and added to ARQ testsuite (I believe @jamezp will be looking at this given he is the original autor and reporter).

@jamezp
Copy link
Collaborator

jamezp commented Jul 8, 2024

@rhusar IIRC during my testing they were both needed. It had something to do with when events are fired and needing different contexts for different events. I am working on some tests in #581. I've only got two so far, but hopefully can get some more this week. Right now it's a single in-container test and a manual mode test.

I'd like to add some tests which use @ArquillianResource's as method parameters too. We know these will fail in JUnit 5, but I'll just link the issue about that as well.

If anyone has any other tests they think would be useful, please let me know.

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