#1308 Switch to JUnit Jupiter and do not use Toolchains for the integration tests#2013
Conversation
There was a problem hiding this comment.
Hey Filip.. this is sure a lot to take in. A really good job to move to Juniper. 💯
I went trough it. Find it a bit tricky to grasp. Perhaps its a good idea to also request @agudian to glance upon this. He's really good in all this junit stuff and perhaps he's got some time. If not, just merge. I think we need to go with this anyway and I can see we scrap a lot of code in favour of standard stuff in juniper.
| /** | ||
| * Adds explicit exclusions of test mappers that are known or expected to not work with specific compilers. | ||
| * | ||
| * @author Andreas Gudian |
There was a problem hiding this comment.
Yes this was within the FullFeatureTest and was written by Andreas
|
|
||
| switch ( currentJreVersion ) { | ||
| case JAVA_9: | ||
| // TODO find out why this fails: |
There was a problem hiding this comment.
Nope, this is an old TODO :), so I left it
| JRE currentJreVersion) { | ||
| List<String> additionalExcludes = new ArrayList<>(); | ||
|
|
||
| // SPI not working correctly here.. (not picked up) |
There was a problem hiding this comment.
should we fix this in a later phase? Make an issue? I guess we rely heavily on SPI.. I guess somewhere the META_INF is not on the classpath.
There was a problem hiding this comment.
This comes from earlier, I didn't write a single new line in this class. Just pulled it outside of the deleted FullFeatureTest. I actually have a better idea now with Jupiter instead of doing what we are doing here.
| * Otherwise, use <code>-Dmapstruct_root=path_to_project</code>. | ||
| */ | ||
| @RunWith( Parameterized.class ) | ||
| @DisabledForJreRange(min = JRE.JAVA_11) |
There was a problem hiding this comment.
cool, I guess this is the trick that allows you to scrap a lot of plumbing..
There was a problem hiding this comment.
Yup this is a Jupiter annotation which disables the test to run on Java 11+. Which is not correct, I should disable it from 12+. But yes it has a lot of good conditions that can be used. Have a look at the package and the other conditions.
|
|
||
| @Test | ||
| public void testUpToDate() throws IOException { | ||
| @ParameterizedTest |
There was a problem hiding this comment.
Oh yes, I love the JUnit Jupiter parametrized tests. They are extremely powerful.
Thanks, I love Jupiter and would slowly move the other tests towards there as well.
The most complex logic (the one using the Verifier) is in In any case I'll write to @agudian to see if he can have a brief look. |
ee95b2a to
49413d4
Compare
…the integration tests The CI should be setup to run on different Java versions, for which the integration tests are going to be run
agudian
left a comment
There was a problem hiding this comment.
Looks great, Filip!
Always nice to be able to delete a lot of code without losing anything! 👍
In a more complex build pipeline, you could even think of building the artifacts with our default JDK version (8?) and use these exact jar files in the integration tests spawned as matrix build. I bet that can be modelled with GitHub actions.
Anyway, getting rid of the toolchains in favor of matrix builds is a good step.
|
Thanks a lot for the review Andreas.
This is actually a great idea and not difficult at all with GitHub Actions. Would put it in the ideas for migrating there. |
The CI should be setup to run on different Java versions, for which the integration tests are going to be run
Travis is currently running Java 8, 11, 13 and EA.
I'll update to GitHub Actions eventually as well
Fixes #1308