[test] Moved all uses of ServiceFactory into integration tests#155
Merged
FelixGV merged 2 commits intolinkedin:mainfrom Jan 6, 2023
Merged
[test] Moved all uses of ServiceFactory into integration tests#155FelixGV merged 2 commits intolinkedin:mainfrom
FelixGV merged 2 commits intolinkedin:mainfrom
Conversation
Also moved all classes under c.l.v.integration.utils from the main sources to the integrationtest sources, within venice-test-common. This means all other modules are now exclusively reserved to unit tests, which brings the unit test run time down to ~20m in the GH CI. Furthermore, adding back a usage of integration utils in those modules should fail at compile time. There is an exception which is the venice-avro-compatibility-test module, since it does need access to integration utils, and it cannot be moved to the integrationtest sources due to needing to run with all Avro versions. Besides the moved files, there is also a bit of moved code in order to accommodate the restructuring. In particular, some functions of the TestPushUtils class are moved into a new IntegrationTestPushUtils class.
FelixGV
force-pushed
the
integration_test_consolidation
branch
from
a447a99 to
6b3beea
Compare
adamxchen
approved these changes
Jan 6, 2023
Contributor
adamxchen
left a comment
adamxchen
left a comment
There was a problem hiding this comment.
Thanks for the change! Left a minor naming comment.
When I saw the unit test for this PR only run 20 mins, I thought the run on my PR had some issues and restarted it. Turned out it was "normal" 😅
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
3 participants
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.
Also moved all classes under c.l.v.integration.utils from the main sources to the integrationtest sources, within venice-test-common. This means all other modules are now exclusively reserved to unit tests, which brings the unit test run time down to ~20m in the GH CI. Furthermore, adding back a usage of integration utils in those modules should fail at compile time.
There is an exception which is the venice-avro-compatibility-test module, since it does need access to integration utils, and it cannot be moved to the integrationtest sources due to needing to run with all Avro versions.
Besides the moved files, there is also a bit of moved code in order to accommodate the restructuring. In particular, some functions of the TestPushUtils class are moved into a new IntegrationTestPushUtils class.
How was this PR tested?
CI
Does this PR introduce any user-facing changes?