Skip to content
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

Fix PowerMock related tests due to version bump #637

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

kaituo
Copy link
Collaborator

@kaituo kaituo commented Aug 9, 2022

Description

2.2 core added new dependencies of ClusterSettings like TaskResourceTrackingService. It is possible that the classloader that loads TaskResourceTrackingService is different from the classloader that loads PowerMock and related dependencies. PowerMock reports java.lang.NoClassDefFoundError when initializing ClusterSettings. Since we are not actually using the value of ClusterSettings in the tests, we make it null to avoid initializing it. The change fixed failed tests.

This PR also fixed spotless errors in FakeNode due to recent changes.

Testing done:

  • gradle build

Signed-off-by: Kaituo Li kaituo@amazon.com

Issues Resolved

#630

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@kaituo kaituo requested a review from a team August 9, 2022 00:29
@opensearch-trigger-bot opensearch-trigger-bot bot added the infra Changes to infrastructure, testing, CI/CD, pipelines, etc. label Aug 9, 2022
@kaituo kaituo requested review from amitgalitz and ohltyler August 9, 2022 00:30
2.2 added a new dependency of ClusterSettings like TaskResourceTrackingService. It is possible that the classloader that loadsTaskResourceTrackingService is different from the classloader that loads PowerMock and related dependencies. PowerMock reports java.lang.NoClassDefFoundError when initializing ClusterSettings. Since we are not actually using the value of ClusterSettings in the tests, we make it null to avoid initializing it. The change fixed failed tests.

This PR also fixed spotless errors in FakeNode due to recent changes.

Testing done:
* gradle build

Signed-off-by: Kaituo Li <kaituo@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2022

Codecov Report

Merging #637 (1aa3c79) into main (f630c8f) will decrease coverage by 0.07%.
The diff coverage is 82.35%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #637      +/-   ##
============================================
- Coverage     79.07%   79.00%   -0.08%     
- Complexity     4213     4237      +24     
============================================
  Files           296      301       +5     
  Lines         17686    17872     +186     
  Branches       1880     1897      +17     
============================================
+ Hits          13986    14120     +134     
- Misses         2805     2848      +43     
- Partials        895      904       +9     
Flag Coverage Δ
plugin 79.00% <82.35%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...opensearch/ad/ratelimit/CheckpointWriteWorker.java 91.25% <0.00%> (-2.58%) ⬇️
...nsearch/ad/ratelimit/RateLimitedRequestWorker.java 71.22% <0.00%> (-0.34%) ⬇️
...search/ad/cluster/ClusterManagerEventListener.java 78.72% <20.00%> (-15.88%) ⬇️
.../org/opensearch/ad/feature/CompositeRetriever.java 81.45% <41.66%> (-3.04%) ⬇️
...nsearch/ad/transport/RCFResultTransportAction.java 85.18% <61.53%> (-0.87%) ⬇️
.../java/org/opensearch/ad/caching/PriorityCache.java 69.38% <65.71%> (-0.87%) ⬇️
...rch/ad/transport/AnomalyResultTransportAction.java 80.87% <66.66%> (+0.05%) ⬆️
...in/java/org/opensearch/ad/caching/CacheBuffer.java 77.69% <75.00%> (-1.17%) ⬇️
.../org/opensearch/ad/ratelimit/ColdEntityWorker.java 90.90% <80.00%> (-4.55%) ⬇️
...rc/main/java/org/opensearch/ad/util/DateUtils.java 80.00% <80.00%> (ø)
... and 26 more

build.gradle Show resolved Hide resolved
Copy link
Member

@ohltyler ohltyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, thanks for the fixes!

Copy link
Member

@amitgalitz amitgalitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for making this fix

@kaituo kaituo merged commit c92cdc8 into opensearch-project:main Aug 9, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 9, 2022
2.2 added a new dependency of ClusterSettings like TaskResourceTrackingService. It is possible that the classloader that loadsTaskResourceTrackingService is different from the classloader that loads PowerMock and related dependencies. PowerMock reports java.lang.NoClassDefFoundError when initializing ClusterSettings. Since we are not actually using the value of ClusterSettings in the tests, we make it null to avoid initializing it. The change fixed failed tests.

This PR also fixed spotless errors in FakeNode due to recent changes.

Testing done:
* gradle build

Signed-off-by: Kaituo Li <kaituo@amazon.com>
(cherry picked from commit c92cdc8)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 9, 2022
2.2 added a new dependency of ClusterSettings like TaskResourceTrackingService. It is possible that the classloader that loadsTaskResourceTrackingService is different from the classloader that loads PowerMock and related dependencies. PowerMock reports java.lang.NoClassDefFoundError when initializing ClusterSettings. Since we are not actually using the value of ClusterSettings in the tests, we make it null to avoid initializing it. The change fixed failed tests.

This PR also fixed spotless errors in FakeNode due to recent changes.

Testing done:
* gradle build

Signed-off-by: Kaituo Li <kaituo@amazon.com>
(cherry picked from commit c92cdc8)
ohltyler pushed a commit that referenced this pull request Aug 9, 2022
2.2 added a new dependency of ClusterSettings like TaskResourceTrackingService. It is possible that the classloader that loadsTaskResourceTrackingService is different from the classloader that loads PowerMock and related dependencies. PowerMock reports java.lang.NoClassDefFoundError when initializing ClusterSettings. Since we are not actually using the value of ClusterSettings in the tests, we make it null to avoid initializing it. The change fixed failed tests.

This PR also fixed spotless errors in FakeNode due to recent changes.

Testing done:
* gradle build

Signed-off-by: Kaituo Li <kaituo@amazon.com>
(cherry picked from commit c92cdc8)
ohltyler pushed a commit that referenced this pull request Aug 9, 2022
2.2 added a new dependency of ClusterSettings like TaskResourceTrackingService. It is possible that the classloader that loadsTaskResourceTrackingService is different from the classloader that loads PowerMock and related dependencies. PowerMock reports java.lang.NoClassDefFoundError when initializing ClusterSettings. Since we are not actually using the value of ClusterSettings in the tests, we make it null to avoid initializing it. The change fixed failed tests.

This PR also fixed spotless errors in FakeNode due to recent changes.

Testing done:
* gradle build

Signed-off-by: Kaituo Li <kaituo@amazon.com>
(cherry picked from commit c92cdc8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport 2.2 infra Changes to infrastructure, testing, CI/CD, pipelines, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants