-
Notifications
You must be signed in to change notification settings - Fork 9k
YARN-9511. Refactoring TestAuxServices to eliminate flakyness #7598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YARN-9511. Refactoring TestAuxServices to eliminate flakyness #7598
Conversation
💔 -1 overall
This message was automatically generated. |
73bde05
to
33d69ee
Compare
Change-Id: I98a827325d646cc1b8c84f01c27e46fe9b3b7d12
33d69ee
to
e23f4bd
Compare
💔 -1 overall
This message was automatically generated. |
Change-Id: I233e00cf93db8207a63acf33f04142997cefcc41
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @p-szucs for this stability improvement! LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch @p-szucs, it's a welcome fix. LGTM, merging to trunk. Could this be backported to maintenance branches (3.4, maybe 3.3)?
Thanks @brumi1024, sure, let me backport this to the maintenance branches as well. |
Change-Id: I98a827325d646cc1b8c84f01c27e46fe9b3b7d12
Description of PR
When we initialize auxiliary services, permission checking iterates through the parents of the manifest file until it reaches the system root, and checks if any of the parents are writable to user group or others. Without mocking this the success of the initialization would heavily depend on the environment where the test is running.
As a solution I mocked AuxServices.checkManifestPermissions method in the test cases where we are assuming that the file system permissions are correct.
How was this patch tested?
Unit tests
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?