-
Notifications
You must be signed in to change notification settings - Fork 29
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
Build on latest Java, test through lowest possible #254
base: master
Are you sure you want to change the base?
Conversation
6530a06
to
baefd37
Compare
Cannot be applied permanently, because Spotless doesn't allow "adding" sources only "setting".
This reverts commit 55b9ebd.
fail-fast: false | ||
max-parallel: 1 | ||
matrix: | ||
jdk: [ 8, 11, 17 ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't got through all the changes, but a quick comment here. E2E tests are quite heavy and they hit Sonatype Nexus. That system "occasionally" has performance problems and I would prefer to not give it an extra load due to our tests. I'm really glad, the Sonatype guys are ok with us testing our e2e processes as some tests, but it's rather for smoke testing (the name could be somehow misleading, in fact they are smoke tests checking the end-to-end process with the real Nexus).
Maybe we could execute the compatTests on that way (with the real binaries)? Thanks to that we would know that JDK 8 execution is still ok, but Nexus would be not affected (which is irrelevant for that test anyway, as the REST calls on the server side shouldn't be JDK specified).
However, I'm not sure, if it would be easy to work with the compatTest to make it possible to execute them in that mode on CI (but still by default with the current JDK 11+, as we rather don't want to force developers to install all JDK 8, 11 and 17 on their machines*). WDYT?
*
- I see nexusPublishPlugin.test.java
which might be helpful for the default (non-CI) mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To reduce the load, we can reduce the matrix for push
, but I think at least right before release it should be executed on all versions via workflow_dispatch. (Conditional CI matrix is possible without too much hassle.)
compatTests are by definition separated by Java versions, the task name contains it.
foojay-resolver does the installing on dev machines as well as CI, so it's not a big deal to have any version. That said I added auto
for devs just to make it more native to the machine.
See https://jakewharton.com/build-on-latest-java-test-through-lowest-java/
Testing: actions executed on this commit show the Java version as failed tests.