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

Added integration tests for JUnit 5 lifecycle methods #314

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lprimak
Copy link
Contributor

@lprimak lprimak commented Feb 23, 2021

Short description of what this resolves:

Integration tests for lifecycle methods with JUnit 5
Relates to #310 #309 and #301

using @TempDir as a base for temporary directory
Could not use it 'as is' because it gets deleted for each test class,
but the files need to be kept throughout the test suite

@lprimak
Copy link
Contributor Author

lprimak commented Feb 23, 2021

Tests will fail until App server tests are set up and #310 is merged

@bartoszmajsak
Copy link
Member

I am not really convinced these tests should live in the core, as they are also dependent on the application server adapter which is a separated entity and can or cannot support particular feature for whatever reason.

My initial thinking is to resurrect arquillian-showcase project and set it up in a way that every PR in arquillian-core would run tests from it as a PR check. Showcase needs work now to be back to life, but I will work on it this week. The next step would be to think about what kind of useful extension example we could build so that JUnit 5 is used there. Alternatively, we can port one or more existing extensions to JUnit 5. On top of that, we could bring Arquillian Chameleon there to easily swap different container adapters and run those tests against them. WDYT?

@lprimak
Copy link
Contributor Author

lprimak commented Feb 23, 2021

I think you are right about all of that. There can easily be some module that does integration testing with a container

@bartoszmajsak
Copy link
Member

I think you are right about all of that. There can easily be some module that does integration testing with a container

Lemme get a bit deeper to it and we will definitely put your valuable to good use :)

@lprimak lprimak force-pushed the JUnit5LifecycleIntegrationTest branch 3 times, most recently from 13a9da2 to dc1fa3b Compare May 5, 2021 02:34
@lprimak
Copy link
Contributor Author

lprimak commented May 5, 2021

@bartoszmajsak I figured out how to do integration tests properly with app servers, so here is the fixed version of this PR.
It passes CI and works properly now. Ready to merge.

@lprimak lprimak changed the title Added integration tests for lifecycle methods Added integration tests for JUnit 5 lifecycle methods May 5, 2021
@lprimak
Copy link
Contributor Author

lprimak commented May 10, 2021

@bartoszmajsak what do you think?

@lprimak lprimak force-pushed the JUnit5LifecycleIntegrationTest branch from ed39c52 to ae95fa4 Compare May 24, 2021 09:54
@lprimak
Copy link
Contributor Author

lprimak commented Jun 20, 2021

@bartoszmajsak
@starksm64

Any chance of getting this merged?

@lprimak lprimak force-pushed the JUnit5LifecycleIntegrationTest branch 3 times, most recently from 3584b11 to eb7e308 Compare June 20, 2021 05:11
@bartoszmajsak
Copy link
Member

bartoszmajsak commented Jun 21, 2021

@lprimak As I mentioned in #314 (comment) I am reluctant to make this part of core as it tests against one specific application server and I would rather have core free of such dependency (even if we are only talking about test dependencies). As suggested in the aforementioned comment, I believe it would be more beneficial to have separated project with such tests and make it part of the verification process in the core module. The challenging bit here is that clearly I don't have capacity to work on this right now :(

@lprimak
Copy link
Contributor Author

lprimak commented Jun 21, 2021

Do you think there is harm to merge this and then later when you have time, you can fix it to your liking?
I personally don't see the harm

@bartoszmajsak
Copy link
Member

Do you think there is harm to merge this and then later when you have time, you can fix it to your liking?
I personally don't see the harm

If there's something I learned the hard way in my software engineering journey it can be summarized as

"There's nothing as permanent as a temporary solution"

I would rather start with something small and separate which can give us an answer "does Arqulian with XUnit work with P Container Adapter" rather than introducing another module to the core. Or maybe even make it part of the container adapter itself? After all, it's a Payara-specific test suite right now.

@lprimak
Copy link
Contributor Author

lprimak commented Jun 21, 2021

"There's nothing as permanent as a temporary solution"

True true, however, there also "iterative development" argument here :)

@lprimak
Copy link
Contributor Author

lprimak commented Aug 7, 2022

@bartoszmajsak I think this is still relevant, can we make some progress here?
thank you!

@lprimak lprimak closed this Nov 19, 2022
@lprimak lprimak reopened this Nov 19, 2022
@lprimak lprimak closed this Nov 19, 2022
@lprimak lprimak reopened this Nov 19, 2022
@lprimak lprimak closed this Nov 19, 2022
@lprimak lprimak reopened this Nov 19, 2022
@lprimak lprimak force-pushed the JUnit5LifecycleIntegrationTest branch from cdbb31c to 88a0e06 Compare May 4, 2023 01:02
@lprimak lprimak force-pushed the JUnit5LifecycleIntegrationTest branch from 88a0e06 to e9268d0 Compare May 4, 2023 04:30
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.

None yet

2 participants