SOLR-18069: Add Maven build smoke test to verify published POMs#4227
SOLR-18069: Add Maven build smoke test to verify published POMs#4227dsmiley wants to merge 8 commits intoapache:mainfrom
Conversation
Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com>
Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> Agent-Logs-Url: https://github.com/dsmiley/solr/sessions/b373120b-17e9-4576-9225-b301ac62afbb
…TestExternalClient Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> Agent-Logs-Url: https://github.com/dsmiley/solr/sessions/eadb1a0d-dacb-4e40-9ea8-ab4bc8371ddf
|
I really like the upping of our game in validating things. I wish we had a more general way of handling lots of kinds of "integration" or "system" tests like this one. We have some emerging tests in our bats infrastructure, and then some of these smokTest things like this. Could this test have been based on a Docker based setup so that if I don't have a specific version of maven and gradle installed, that it would still work? It would be nice if running this was a constant thing, not a "and now it's release time" type thing. I also am not sure about more and more directories at the root.. Could |
Wrt maven files, it is most crucial to avoid shipping something broken, so I'm good with release-time. But I also agree that if it was wrappable in a gradle target, then any change to gradle files could trigger such a test in github actions. Since it involves actually running |
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
Note there are multiple Jenkins CI jobs for the smoketester: https://ci-builds.apache.org/job/Solr/job/Solr-Smoketest-10.x/ -- that one is disabled for some reason.
The bit of Docker here was only so that we don't add an extra requirement to have Maven installed. Maybe I could add the "Maven Wrapper" instead. Or I could just remove that and it becomes optional (only run when mvn is on the path). After all, most envs that have Gradle will have Maven as well. BTW I didn't test this Docker apsect whatsoever.
Jan/Eric you both suggest more Docker usage for this particular test-external-client but I don't see the point as plainly it works without such. I don't think we should use it on the grounds for testing additional Maven/Gradle versions -- IMO that's overkill and additional maintenance burden.
| @@ -83,10 +83,13 @@ static Path determineSourceHome() { | |||
| } | |||
|
|
|||
| Path base = file.toAbsolutePath(); | |||
| while (!Files.exists(base.resolve("solr/test-framework/build.gradle")) && null != base) { | |||
There was a problem hiding this comment.
This is a subtle difference that I bet nobody will notice. Not the null check but the path. Notice I removed the leading "solr/" component, and then some lines below here, I removed adding the "solr/" back on when we return, as it becomes pointless. On one hand, this subtle difference is a micro-optimization -- why iterate to the root of the project when we can iterate and stop at the "solr/" child and then reach success one iteration sooner. My real motivation is that this is absolutely critical for test-external-client, which uses ExternalPaths. We want that project to experience what any project outside Solr would see -- null for all paths here. So @epugh that answers why it's critical that test-external-client be at the root, or at least not inside solr/.
|
I could see that Docker could be used to execute the build of this little project in order to provide total detachment/isolation from the source tree, thus making it needless for me to be a little clever in ExternalPaths regarding the leading "solr/". |
|
BTW I generalized the scope. The PR title suggests this is only about published POMs. That is one thing this little project indirectly checks, the other thing that is checked is that solr-test-framework can actually be used externally. Hence I called this dir "test-external-client". |
https://issues.apache.org/jira/browse/SOLR-18069
Original PR in my fork done by CoPilot Agent: dsmiley#21
Didn't test/run this and I don't know what to think of the code. This is just a draft PR after all ;-)
I love that the maven aspects are factored out and individually runnable.
TBH I'm the last one to be writing python here. Ideally someone immediately takes over this. Or tells me to close this pile of crap ;-)