Updating tables-test-fixture uber jar to not have any unshaded dependencies #233
Conversation
This file contains 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
maluchari
force-pushed
the
malini/test_tt_2
branch
from
b4082b0
to
3bf9264
Compare
HotSushi
reviewed
Oct 17, 2024
HotSushi
approved these changes
Oct 18, 2024
17 tasks
maluchari
added a commit
that referenced
this pull request
Oct 31, 2024
## Summary This PR adds back some exclusions and undoes some relocations that were done as part of PR #233. The uber jar contains some additional dependencies but minimum ones that are required to run tables-test-fixtures test. <img width="337" alt="Screenshot 2024-10-30 at 1 53 01 PM" src="https://github.com/user-attachments/assets/d599bbb6-7a38-46e6-bfc2-2d2f627852f0"> Reasons for retaining the jars: 1. Relocation or exclusion of hibernate, ehcache, h2 makes dependencies to be unavailable for hts as part of the tests. 2. Relocation or exclusion of zaxxer makes wrong primary bean to be picked up for hts as part of the tests. 3. javaxx relocation or exclusion causes class not found errors as configs are required to be overridden in tomcat which inturn cant be relocated without springframework relocation. Same applies for springdoc relocation and io.micrometer as well Keeping the above out of scope for this PR and can be explored further to identify the right set of exclusions or relocations that need to happen together. ## Changes - [ ] Client-facing API Changes - [ ] Internal API Changes - [ ] Bug Fixes - [ ] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [X] Tests For all the boxes checked, please include additional details of the changes made in this pull request. ## Testing Done <!--- Check any relevant boxes with "x" --> - [ ] Manually Tested on local docker setup. Please include commands ran, and their output. - [ ] Added new tests for the changes made. - [ ] Updated existing tests to reflect the changes made. - [ ] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [X] Some other form of testing like staging or soak time in production. Please explain. Tested with snapshot jar and a new module that Sushant provided. For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [ ] Large PR broken into smaller PRs, and PR plan linked in the description. For all the boxes checked, include additional details of the changes made in this pull request.
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.
Summary
The tables-test-fixture uber jar brought in a lot of unrelocated dependencies some of which are unnecessary but caused classpath conflicts while leveraging it for testing. So the objective of this PR is to
The exclusions and relocations were identified through a combination of trial and error and through dependencyInsights.
springframework is still pending relocation and is kept out of scope for this PR. This will be addressed if there is a need for the same. This specific relocation caused autowired beans to be not recognized despite merging or appending the transformed properties or by modifying spring.factories file. Added a TODO to address it if it comes up as an issue.
Changes
Testing by depending on this jar on another product and ensuring the tests go through.
For all the boxes checked, please include additional details of the changes made in this pull request.
Testing Done
Jars that are present post exclusion and relocation
Additional Information
For all the boxes checked, include additional details of the changes made in this pull request.