Rework ci pipeline

[juherr]

Based on #1599

This PR restructures the CI in the following ways:

1. Execution of integration tests for jsonschema2pojo-gradle-plugin has been removed.
2. Build artifacts are now shared, so other jobs can test against them.
3. The 3 example projects are built against the shared artifacts.

[ctrimble]

Thanks for isolating this part of #1599 . The fact that the project is waiting for Test JDK 8 as a required check points out that we are breaking the way the repo is configured.

The configuration for what checks must pass is not under my control. I think we should conform to the existing naming convention. Let's apply the principal of least surprise to Joe's benefit, expanding on his existing naming conventions.

Names like these will work:

• Test JDK {VERSION} for cases where we are testing the main build against a version.
• JDK {VERSION} | Example {BUILD_TOOL} (TARGET_PLATFORM)

When we expand this to other JDKs in another PR, I think we should be careful not to create too much noise. We should:

1. Test the main build against each LTS version.
2. Test the examples against each LTS version, but we should avoid a matrix of build and run versions.

That should keep our footprint reasonable while still establishing that things work against these LTS version.

[ctrimble]

I think the name is backwards. We should have the run.java version in here, since we are testing that we can run the example against our built artifacts.

[joelittlejohn]

Execution of integration tests for jsonschema2pojo-gradle-plugin has been removed.

Could you explain this change @juherr? How are these tests run now?

[ctrimble]

Thanks @joelittlejohn for hopping in. This is why I updated the description.

It looks like #1544 added a special mvn install and integration test for the gradle plugin. I am digging into whether we are actually dropping this integration test run or if we were preventing it from running twice with this change.

[ctrimble]

@juherr actions is complaining about your last change.

[Invalid workflow file: .github/workflows/ci.yml#L1](https://github.com/joelittlejohn/jsonschema2pojo/actions/runs/18618519878/workflow)
(Line: 53, Col: 11): Unrecognized named-value: 'project-java-target'. Located at position 1 within expression: project-java-target, (Line: 90, Col: 11): Unrecognized named-value: 'project-java-target'. Located at position 1 within expression: project-java-target, (Line: 124, Col: 11): Unrecognized named-value: 'project-java-target'. Located at position 1 within expression: project-java-target

[ctrimble]

Looking at output from recent builds, the gradle plugin module's failsafe plugin is not bound to the verify phase for some reason, but the integration module's failsafe plugin is. This change does drop a failsafe execution.

[juherr]

Could you explain this change @juherr? How are these tests run now?

The integration test was originally used just to verify that the example could build, which wasn’t straightforward and only worked for one out of three examples.
I’ve switched to running all of them directly in CI instead of through Maven.

[joelittlejohn]

Okay, sounds good. Thanks for the explanation.

[juherr]

Everything looks good now.

Looking at output from recent builds, the gradle plugin module's failsafe plugin is not bound to the verify phase for some reason, but the integration module's failsafe plugin is. This change does drop a failsafe execution.

I'm not sure what is expected here, but it doesn't seem related to this pull request.

[juherr]

I finally got what you meant and removed the file properly. As you can see, I was just building the example.

[joelittlejohn]

Is it necessary to remove this test? Ideally if there is some change happening in the Gradle plugin, then we want that to be tested locally before pushing to GitHub (that's a long feedback loop). The current integration test is a good sanity check that the Gradle plugin hasn't broken.

[joelittlejohn]

I'm open to alternatives if there's a bette way.

[ctrimble]

I would really like to see us keep that test in the test suite. You can't start a debugging session from a CI only test. Disabling it under later JDKs would be better than dropping it.

As for the failsafe plugin not running, I am sure that is some kind of misconfiguration. #1554 indicates that it was placed on the install phase, but doesn't say way. It may just be misconfiguration. We could fix that in another PR.

[joelittlejohn]

I think you'll find that the test doesn't use the Gradle plugin version that has just been built and is in the reactor, it uses the version from your local repository. Attaching this to install allowed the actual latest built plugin to be tested (if you use mvn install).

[ctrimble]

OK, so in verify we have the artifacts sitting in the target directory, but they are not in .m2/repository yet. So, the test was pushed to install, allowing the artifacts to get moved and now be on the gradle classpath.

[ctrimble]

I just took a little time to get my head around this. I was able to get the artifact from the target directory into the gradle example using fileTree, but it was hacky and brittle. Not a good solution.

There is a TestKit provided by gradle. Perhaps we should:

1. Leave the test itself as is, but mark it as JDK 8 only. That will leave us a tight feedback loop.
2. Let the special logic in the CI get removed in favor of tests based on examples. There is value in that.
3. Rework the test later with TestKit, so that failsafe doesn't sit in this strange location in the lifecycle.

[juherr]

Should I restore the removed test then?

I’m still not sure I understand the rationale for using the example as an integration test.
If that's a key requirement, could you explain why it’s not yet done in the Maven plugin?

[ctrimble]

Here is the rational: #1554 exposed an issue with generation, so the test the project thought was checking the current plugin was shifted around to make it work during a standard mvn clean install type build. Going back and forth with an issue using the CI is painful, so Joe is pointing out that moving this just to the CI could open the project back up to hard to fix issues. I agree, having dealt with similar situations in the past.

Personally, I don't think it matters which check, or both, runs in the CI. What is important is that tight feedback loop.

Knowing that the test will fail with #1599, we will need to do something. Unless @joelittlejohn objects, I think we should:

1. Let the CI configuration for this go, in favor of these new checks. They will catch the same issues.
2. Leave the test in as is and skip it for JDKs other than 8. That will keep it in the build when we need it.
3. Try to replace the test with TestKit in another PR and get it back into maven's verify phase.

Thoughts?

[joelittlejohn]

@juherr Yes, please restore the test, even if you have to ignore it. It's really nice to have some basic good sanity check of the Gradle plugin before you push. If you have to ignore it for now that's fine. We can revisit later and find the best way to test.

[juherr]

@ctrimble @joelittlejohn Test restored.
I suggest merging this first and letting you revisit the integration tests later.
Once it’s merged, I’ll rebase #1599.

[juherr]

Any update about this pr?