-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
NIFI-11440: Speed up Hive Metastore based unit tests #7170
Conversation
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 optimization @mark-bathori! The general approach looks good. The only problem is that the ThriftMetastore.beforeAll()
method fails on Windows due to a path issue. It would be help to correct this so that the test can run on all platforms, but if that is not an option, the best approach seems to be promoting the DisabledOnOs
annotation to the class level.
I would advise not to waste any time bothering with these tests on Windows. I'd just detect running on windows and ignore them. |
I concur, the tests are already disabled on Windows, so promoting the annotation from method level to class level seems like the best path forward. |
Thanks @exceptionfactory for the review. I overlooked that the modified |
Thanks for making the adjustments @mark-bathori! I started the automated build and will plan on merging pending successful completion on all platforms. |
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.
Build success on all platforms, thanks @mark-bathori! +1 merging
Summary
NIFI-11440
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
LICENSE
andNOTICE
filesDocumentation