java
java copied to clipboard
paketo-buildpacks
Add initial set of integration tests
Summary
As briefly discussed on Slack with @dmikusa, this is an initial set of integration tests run for each PR (and main). It mainly should trigger discussions around a testing strategy. The currently active tests add ~142s to the build time.
- When should integration rests run?
- Which integration tests should run?
- Split tests (have different sets) for PRs and releases?
- etc.
The TomEE tests are currently disabled due to https://github.com/paketo-buildpacks/apache-tomee/pull/96, which we discovered while working on the integration tests.
Use Cases
Checklist
- [x] I have viewed, signed, and submitted the Contributor License Agreement.
- [x] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
- [x] I have added an integration test, if necessary.
- [x] I have reviewed the styleguide for guidance on my code quality.
- [x] I'm happy with the commit history on this PR (I have rebased/squashed as needed).
CC @loewenstein
When should integration rests run?
We should for sure have them all run before doing a release. I'm not sure if GHA has that exact capability, but something like that. The idea is to give confidence that the impending release is solid.
Which integration tests should run? Split tests (have different sets) for PRs and releases?
My concern with running tests on PR or merge is that this repo will get a lot of small PRs to update the buildpack dependencies (one per buildpack). It seems unnecessary to run all the tests against every PR. If we could possibly run specific tests paired with certain buildpack updates, that might be interesting. i.e. if spring-boot updates, then we run the spring boot test. Again, I'm not sure how feasible that is with GHA.
We should for sure have them all run before doing a release. I'm not sure if GHA has that exact capability, but something like that. The idea is to give confidence that the impending release is solid.
There are three options I think:
- Run on each merge on
master: Would slow down smaller PRs, but better than running for each PR. - Run on new tags: Might be a bit late i.e. after releasing the draft
- Run manually: Before releasing the draft or have a workflow that first executes the integration tests and then trigger the release via API.
We'd prefer option 1 but I guess that option 3 would be closer to your anticipated scenario.
Which integration tests should run?
We currently added matrix tests for spring-boot, Tomcat and TomEE. Would that be sufficient (or too much) for a first contribution?
Apologies, I haven't had a chance to look at this a ton. I like the tests you have setup, although the one is commented out. The test scenarios top of my mind are Spring Boot as executable jar, Spring Boot as native image, Tomcat, Tomee, and Liberty.
Im still not sure as to when we run them. On PR and on merge, seem like they would be too frequent. Releases for components are usually cut Wed or Thu, and that causes a lot of individual PRs to this repo. I'm worried this will slow down the process as well as just create a lot of unnecessary CI runs, possibly causing us to hit limits.
I'd like to get some thoughts from @pivotal-david-osullivan as he does most of the merge and release work now, and also he might have some opinions about how we could modify the release process to incorporate running the tests before we cut the release. Perhaps the answer is to have the integration tests manually triggered and integrate that into the release process, ie trigger it, wait for a pass then cut the release.
What do you think @pivotal-david-osullivan ?
@dmikusa & @pivotal-david-osullivan Did you get a chance to exchange opinions?
I've looked into another potential option for running these tests and I think it could work as follows:
- We have all new component buildpack dependency bump PRs open against a new 'draft' branch, rather than main
- When all these are ready we merge into main from this draft/staging branch.
- We then run the tests on merges to main
It would require a change to the CI & release process so I will open an RFCs if this sounds feasible?
I'm 👍 for this approach. It doesn't block the release if there are integration problems, but it a.) runs the tests automatically and b.) doesn't excessively run the tests.
Unrelated, I think that using a draft branch could also offer other potential benefits, like auto-merge PRs to bump component buildpack versions in that branch. That would still give us the opportunity to review those changes before committing to main, but it would save a lot of time merging those PRs.
I do think we should go through the RFC process for this, apologizes as that will take longer. Doing the RFC will increase transparency, which I think is important since this is a core workflow change, and I know that there are some users of pipeline-builder so they should have a chance to comment on this proposed change before its made.
- We have all new component buildpack dependency bump PRs open against a new 'draft' branch, rather than main
Just to clarify, that would mean that the integration tests can run for every PR to main, right?
- We have all new component buildpack dependency bump PRs open against a new 'draft' branch, rather than main
Just to clarify, that would mean that the integration tests can run for every PR to main, right?
Yes, that's correct.
There would basically be one PR to main per week. All of the component update PRs would go into a draft branch, at the end of the week that would be reviewed & merged into main. The integration tests would run & when they pass the release could be cut as it is now.
There are occasionally PRs to main where we might change buildpack metadata, like the order group. Those would trigger integration tests as well.
Apologies @modulo11, I merged this by accident. it's been reverted but I cannot re-open it, can you try pushing a commit to the branch and opening a PR from it?