Skip to content

Refactor plugin system index tests to use parameterized test pattern#5895

Merged
cwperks merged 6 commits intoopensearch-project:mainfrom
llilyy:refactor/p-test
Jan 12, 2026
Merged

Refactor plugin system index tests to use parameterized test pattern#5895
cwperks merged 6 commits intoopensearch-project:mainfrom
llilyy:refactor/p-test

Conversation

@llilyy
Copy link
Contributor

@llilyy llilyy commented Jan 5, 2026

Description

  • Category
    Refactoring

  • Why these changes are required?
    Reviewer feedback suggested adopting the parameterized test suite pattern used in IndexAuthorizationReadOnlyIntTests.java to improve test organization, reduce duplication, and ensure consistent behavior across different cluster configurations.

  • What is the old behavior before changes and new behavior after changes?
    Introduced a new PluginSystemIndexIntTests.java class that uses JUnit's Parameterized runner.

Issues Resolved

#5893

Testing

./gradlew :integrationTest --tests "org.opensearch.security.privileges.int_tests.PluginSystemIndexIntTests"
# BUILD SUCCESSFUL

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • Commits are signed per the DCO using --signoff

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.

Signed-off-by: llilyy <llilyy0215@gmail.com>
@nibix
Copy link
Collaborator

nibix commented Jan 6, 2026

Thanks a lot for the changes. We have test failures, but I am pretty sure that these will be fixed as soon as #5865 is merged.

llilyy added 2 commits January 8, 2026 10:22
Signed-off-by: llilyy <llilyy0215@gmail.com>
Signed-off-by: llilyy <llilyy0215@gmail.com>
@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.69%. Comparing base (70e03b2) to head (31222dd).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5895      +/-   ##
==========================================
+ Coverage   73.67%   73.69%   +0.01%     
==========================================
  Files         437      437              
  Lines       26655    26666      +11     
  Branches     3945     3947       +2     
==========================================
+ Hits        19639    19651      +12     
- Misses       5141     5143       +2     
+ Partials     1875     1872       -3     

see 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@llilyy
Copy link
Contributor Author

llilyy commented Jan 8, 2026

@nibix
I've resolved the conflicts. Could you please review it?

nibix
nibix previously approved these changes Jan 8, 2026
Copy link
Collaborator

@nibix nibix left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@llilyy
Copy link
Contributor Author

llilyy commented Jan 8, 2026

Hi, @cwperks!
It needs one more approval to proceed. As you've been following this thread, could you please review it?

@cwperks
Copy link
Member

cwperks commented Jan 8, 2026

@llilyy minor comment on the javadoc, but otherwise lgtm.

Signed-off-by: llilyy <llilyy0215@gmail.com>
@llilyy
Copy link
Contributor Author

llilyy commented Jan 12, 2026

@nibix
The previous approval was removed due to recent code changes. Could you please take another look? Thanks for your help!

Copy link
Collaborator

@nibix nibix left a comment

Choose a reason for hiding this comment

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

Approved ...

I have retriggered one failed CI job, hopefully it will be green soon.

@cwperks cwperks merged commit ab77299 into opensearch-project:main Jan 12, 2026
69 of 70 checks passed
@llilyy llilyy deleted the refactor/p-test branch January 12, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants