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

REST API tests refactoring (Part 2) #4252

Merged
merged 1 commit into from
May 1, 2024

Conversation

willyborankin
Copy link
Collaborator

@willyborankin willyborankin commented Apr 15, 2024

Description

Changes:

  • Moved test SslCertsApiTest into SslCertsRestApiIntegrationTest
  • Fixed tests framework certificates generation for nodes.

Tests with the Legacy prefix removed since
new tests use randomization for paths

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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.

@willyborankin willyborankin changed the title Moved tests into integration tests (Part 2) REST API tests refactoring (Part 2) Apr 15, 2024
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.05%. Comparing base (6d35a0a) to head (dda9054).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4252      +/-   ##
==========================================
- Coverage   66.12%   66.05%   -0.07%     
==========================================
  Files         302      302              
  Lines       21758    21758              
  Branches     3522     3522              
==========================================
- Hits        14387    14372      -15     
- Misses       5609     5618       +9     
- Partials     1762     1768       +6     
Files Coverage Δ
...arch/security/ssl/OpenSearchSecuritySSLPlugin.java 86.30% <ø> (ø)

... and 6 files with indirect coverage changes

@willyborankin willyborankin changed the title REST API tests refactoring (Part 2) Redesigne /ssl/certs REST API endpoint Apr 23, 2024
@willyborankin willyborankin added breaking This issue is or proposes a breaking change backport 2.x backport to 2.x branch labels Apr 23, 2024
@willyborankin willyborankin force-pushed the refactor-tests#2 branch 2 times, most recently from b5de9cb to a0a407c Compare April 23, 2024 15:53
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

It would be helpful to me if you worked backwards from the documentation-website to show what is getting removed and replaced. This would help me answer the question is this cohesive across how we consider certs.

What do you think?

@willyborankin willyborankin force-pushed the refactor-tests#2 branch 2 times, most recently from 1d88ef7 to 74dfc4b Compare April 29, 2024 12:15
@willyborankin willyborankin changed the title Redesigne /ssl/certs REST API endpoint REST API tests refactoring (Part 2) Apr 29, 2024
@willyborankin willyborankin removed the breaking This issue is or proposes a breaking change label Apr 29, 2024
@willyborankin
Copy link
Collaborator Author

willyborankin commented Apr 29, 2024

In order to do not introduce breaking changes. Added a new endpoint instead in this PR: #4299

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

LGTM. Left a clarifying question in a comment.

reta
reta previously approved these changes May 1, 2024
Changes:
 - Moved testSslCertsApiTest into SslCertsRestApiIntegrationTest
 - Fixed tests framework certificates generation for nodes.

Tests with the Legacy prefix removed since
new tests use randomization for paths

Signed-off-by: Andrey Pleskach <[email protected]>
@willyborankin willyborankin dismissed stale reviews from reta and DarshitChanpura via dda9054 May 1, 2024 11:58
@willyborankin willyborankin removed the backport 2.x backport to 2.x branch label May 1, 2024
@reta reta added the backport 2.x backport to 2.x branch label May 1, 2024
@reta reta merged commit f130915 into opensearch-project:main May 1, 2024
82 of 83 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-4252-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f130915daa969643c3996e0500e2bdbf3499c674
# Push it to GitHub
git push --set-upstream origin backport/backport-4252-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-4252-to-2.x.

dlin2028 pushed a commit to dlin2028/security that referenced this pull request May 1, 2024
willyborankin added a commit to willyborankin/security that referenced this pull request May 7, 2024
Signed-off-by: Andrey Pleskach <[email protected]>
(cherry picked from commit f130915)
@willyborankin willyborankin deleted the refactor-tests#2 branch June 14, 2024 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants