smokeTestRelease.py: fix SNAPSHOT local testing#4556
Open
dsmiley wants to merge 4 commits into
Open
Conversation
Split verifyUnpacked into verifySrcUnpacked and verifyBinaryUnpacked, removing the content-presence checks for the source release (the build failing is sufficient signal). Also extracts unpack() so call sites explicitly pair unpack + the appropriate verify function. Removes unused args from each verify function and trims dead code. Misc fixes enabling local smoke testing against SNAPSHOT builds: - Guard GPG setup/teardown behind isSigned (skip when --not-signed) - Skip KEYS download when not signed and no local keys provided - Skip Changes.html version check for SNAPSHOT versions - Strip -SNAPSHOT from Specification-Version JAR manifest check - Fix X-Build-JDK trailing dot - Skip verifyDeployedPOMsCoordinates for SNAPSHOT (timestamp paths differ from coordinate) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dsmiley
commented
Jun 27, 2026
| 'X-Compile-Source-JDK: %s' % compileJDK, | ||
| 'X-Compile-Target-JDK: %s' % compileJDK, | ||
| 'Specification-Version: %s' % version, | ||
| 'X-Build-JDK: %s.' % BASE_JAVA_VERSION, |
Contributor
Author
There was a problem hiding this comment.
that period is weird... shouldn't be there
Comment on lines
-625
to
-633
| # if not isSrc: | ||
| # # TODO: we should add verifyModule/verifySubmodule (e.g. analysis) here and recurse through | ||
| # expectedJARs = () | ||
| # | ||
| # for fileName in expectedJARs: | ||
| # fileName += '.jar' | ||
| # if fileName not in l: | ||
| # raise RuntimeError('solr: file "%s" is missing from artifact %s' % (fileName, artifact)) | ||
| # in_root_folder.remove(fileName) |
Contributor
Author
There was a problem hiding this comment.
IMO not worth doing this TODO so I removed. Testing ought to require the features in those underlying JARs.
Comment on lines
-635
to
-645
| if isSrc: | ||
| expected_src_root_folders = ['build-tools', 'changelog', 'dev-docs', 'dev-tools', 'gradle', 'solr'] | ||
| expected_src_root_files = ['build.gradle', 'gradlew', 'gradlew.bat', 'settings.gradle', 'settings-gradle.lockfile', 'versions.lock'] | ||
| expected_src_solr_files = ['build.gradle'] | ||
| expected_src_solr_folders = ['benchmark', 'bin', 'modules', 'api', 'core', 'cross-dc-manager', 'docker', 'documentation', 'example', 'licenses', 'packaging', 'distribution', 'server', 'solr-ref-guide', 'solrj', 'solrj-jetty', 'solrj-streaming', 'solrj-zookeeper', 'test-framework', 'webapp', '.gitignore', '.gitattributes'] | ||
| is_in_list(in_root_folder, expected_src_root_folders) | ||
| is_in_list(in_root_folder, expected_src_root_files) | ||
| is_in_list(in_solr_folder, expected_src_solr_folders) | ||
| is_in_list(in_solr_folder, expected_src_solr_files) | ||
| if len(in_solr_folder) > 0: | ||
| raise RuntimeError('solr: unexpected files/dirs in artifact %s solr/ folder: %s' % (artifact, in_solr_folder)) |
Contributor
Author
There was a problem hiding this comment.
not doing this anymore; an earlier comment justifies why.
janhoy
approved these changes
Jun 27, 2026
Contributor
There was a problem hiding this comment.
Sounds useful to also be able to run smoke tester on SNAPSHOT, and nice optimizations. I guess we can live with the reduced checks for files/folders in src release, I see you kept the checks for binary release which I think is good. Kicked off a Copilot check just to catch potential refactoring bugs.
EDIT I withdraw my approval until copilot reviews are dealth with. In particular make sure to test that alt_javas feature works as expected
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates smokeTestRelease.py to better support local smoke testing against -SNAPSHOT builds and refactors the unpack/verify flow by separating source vs binary artifact verification.
Changes:
- Split artifact verification into
verifySrcUnpackedvsverifyBinaryUnpacked, and extractunpack()so callers explicitly pair unpack + the right verifier. - Adjust validation behavior for SNAPSHOTs (skip Changes.html “Release X.Y.Z” check, normalize
Specification-Version, skip deployed POM filepath/coordinate checks). - Improve
--not-signedbehavior by avoiding KEYS download and guarding GPG setup/teardown behindisSigned.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix TypeError in OpenAPI missing-artifact error (stray % version with no placeholder). Fix alt-Java loops in verifySrcUnpacked and verifyBinaryUnpacked passing runner function instead of JAVA_HOME path to testSolrExample. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split verifyUnpacked into verifySrcUnpacked and verifyBinaryUnpacked, removing the content-presence checks for the source release (the build failing is sufficient signal). Also extracts unpack() so call sites explicitly pair unpack + the appropriate verify function. Removes unused args from each verify function and trims dead code.
Misc fixes enabling local smoke testing against SNAPSHOT builds: