All active tests transformed from JUnit1-4 to Junit5 #23598
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- also moved "example" impl to own package and formatted definitions so now it makes a bit more sense (to me at least)
- jdk 1.5 is not supported any more
- also moved test example code and utilities to own package
…crazy stuff - many tests were ignoring errors, so I removed catch blocks - some tests were not closing resources - some patrs are simply obsoleted and should be deleted - some dead code was deleted - this was just a quick visit, more work required to reach much better quality
…junit4 - and also doesn't swallow exceptions any more
- also deleted ProxyModule - wasn't used anywhere
- some tests were bit extended, some were deleted
- based on hk2-junitrunner - portions of code were copy pasted as they are not public - public classes were reused - simplified own integration with tests respecting JUnit5 principles
- InteractiveTester should be transformed to real TestContainers test later.
…stead of junit4
- tests were enhanced a bit
…+ deleted zombies - two test classes were commented out and not usable
- referred file doesn't exist any more
…of junit4 + enhanced - one_probe.xml contained two probes and two_probe.xml just one - test didn't test anything, just loaded files without exceptions
…l experience - prepareAdminSubject - this heuristic didn't work and always ended in the block in the end of the method. That was moved to Utils.createInternalAsadminSubject. Somewhere in the progress I used it in the extension until I found better and more stable solution. - HK2Extension now allows to use two own annotations - ExcludeClasses disables tries to create services of given classes - CustomConfiguration allows to use given domain.xml file for test
- important to have the best possible information about the failure
…led and fixed tests - lesson: whatever you will annotate with @service will be discovered - lesson: whatever annotated with @secure cannot be instantiated in tests - lesson: @secured services can be mocked - resolution: ConfigApiTest must die, HK2Junit5Extension must be extensible to allow customizations for the module (own dependencies) - tests were enabled, fixed and extended, I hope they are stable enough - mocks and examples were moved to the package based on the name of the module - several classes in tests removed, because they weren't used. - DomainTest.xml was updated - based on real domain.xml from current GlassFish 6.2.1 Signed-off-by: David Matějček <dmatej@seznam.cz>
…unit4 + enabled some tests
…junit4 - DefaultGUICallbackHandlerTest can be executed in Eclipse, depends on AWT UI - several tests enabled - AppClientContainerTest deleted, depended on unknown classes and was commented out
… 10 is equal to 10
- ServerEnvironmentImpl is extremely problematic class as it's initialization is not well defined, but the class changes System.properties and affects whatever uses them later (but doesn't affect what started earlier)
- added FIXME for ServerEnvironment, see issue eclipse-ee4j#23593 on GitHub
- resolved cyclic dependencies and side effects (at least partially, so now tests pass in maven
- Replaced by the Junit5 extension - also fixed several tests which never did cleanup
- moved methods and classes - remains of Utils were renamed to ReflectionUtils as it is the only thing left - ReflectionUtils replaced mockit.Deencapsulation, which was removed from jmockit as a bad practice. Same applies here, the less you will use it, the better code you have. But in GF case there are few places which cannot be tested in better way now. Yet.
dmatej
commented
Sep 9, 2021
| @Inject | ||
| private StartupContext startupContext; | ||
|
|
||
| // FIXME: Workaround, because ServerEnvironmentImpl changes global System.properties, but other services |
There was a problem hiding this comment.
Based on this I created this issue: #23593
- formatting - trivialities - renamed AppTest to JavaEETransactionManagerTest (it tests behavior of the class)
|
Passed with no problem on I've only checked that in code just tests were modified, and that junit4 was removed from nucleus/parent. |
arjantijms
approved these changes
Sep 9, 2021
|
Forgot to post the versions: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes were limited to src/test directories and test-utils module.
Main changes:
Notes for reviewers
Please run the build including tests on your machine and report if it would fail - which module, which tests and don't forget to mention your system, JDK version and please, add stacktrace.
I am especially curious if there would not be any issues on Windows or Mac.
mvn clean install -Pstaging -T4C -PjacocoTested on: