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.

Sign up for GitHub

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

Jump to bottom

operator/pkg/tasks: unit test apiserver #5624

Merged
merged 1 commit into from
Oct 12, 2024
Merged

operator/pkg/tasks: unit test apiserver #5624

merged 1 commit into from
Oct 12, 2024

Conversation

mohamedawnallah
Copy link
Contributor

@mohamedawnallah mohamedawnallah commented Sep 28, 2024 •
edited
Loading

Description

In this commit, we introduce unit tests for the apiserver init task in the operator pkg by testing the running and waiting operations on Karmada API Server and Karmada Aggregated API Server.

What type of PR is this?

Which issue(s) this PR fixes:
Part of #5491.
Depends on #5630.

Does this PR introduce a user-facing change?:

NONE

@karmada-bot karmada-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 28, 2024
@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch from 64fdc30 to 2d5cda3 Compare September 28, 2024 10:46
@mohamedawnallah mohamedawnallah mentioned this pull request Sep 28, 2024
Open
@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch 2 times, most recently from 6e4e958 to f5e8cd0 Compare September 28, 2024 10:58
@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2024 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 73.80952% with 11 lines in your changes missing coverage. Please review.

Project coverage is 36.57%. Comparing base (c16e52a) to head (ecdc909).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
operator/pkg/util/apiclient/test_helpers.go 73.80% 8 Missing and 3 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5624      +/-   ##
==========================================
+ Coverage   35.85%   36.57%   +0.72%     
==========================================
  Files         647      649       +2     
  Lines       45070    45121      +51     
==========================================
+ Hits        16158    16503     +345     
+ Misses      27664    27342     -322     
- Partials     1248     1276      +28
Flag Coverage Δ
unittests 36.57% <73.80%> (+0.72%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch from f5e8cd0 to 6b0fb06 Compare September 30, 2024 19:56
@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 30, 2024
@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch 2 times, most recently from be3ec7d to 3f12649 Compare October 6, 2024 01:52
@XiShanYongYe-Chang
Copy link
Member

Hi @mohamedawnallah, the ci lint is failed, can you help fix it?

@mohamedawnallah
Copy link
Contributor Author

@XiShanYongYe-Chang This PR depends on #5630 that needs to be merged first.

@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch from 3f12649 to 70bebd2 Compare October 9, 2024 04:10
@mohamedawnallah
Copy link
Contributor Author

Rebased now on master branch after #5630 PR got merged. I will ask for review when CI GitHub tests finish :)

@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch 2 times, most recently from c4de1d8 to 5cbf05c Compare October 9, 2024 04:35
@mohamedawnallah
Copy link
Contributor Author

cc: @XiShanYongYe-Chang All tests passed now after rebasing :)

@XiShanYongYe-Chang
Copy link
Member

Thanks~ @mohamedawnallah
/assign

XiShanYongYe-Chang
XiShanYongYe-Chang reviewed Oct 9, 2024
View reviewed changes
Copy link
Member

@XiShanYongYe-Chang XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

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

LGTM
/cc @chaosi-zju

@mohamedawnallah mohamedawnallah mentioned this pull request Oct 9, 2024
Merged
@mohamedawnallah
operator/pkg/tasks: unit test apiserver
ecdc909 
In this commit, we introduce unit tests for the apiserver init
task in the operator pkg by testing the running and waiting operations
on Karmada API Server and Karmada Aggregated API Server.

Signed-off-by: Mohamed Awnallah <[email protected]>
@mohamedawnallah mohamedawnallah force-pushed the unitTestAPIServerTaskInitOperatorPKG branch from 5cbf05c to ecdc909 Compare October 10, 2024 04:55
@karmada-bot karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2024
@mohamedawnallah mohamedawnallah mentioned this pull request Oct 10, 2024
Open
@mohamedawnallah
Copy link
Contributor Author

mohamedawnallah commented Oct 12, 2024 •
edited
Loading

Cc @XiShanYongYe-Chang, there are some PRs blocked because they depend on the merging of this PR since it includes some utilities. Could we get this in sooner?

@XiShanYongYe-Chang
Copy link
Member

OK.

/approve

@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: XiShanYongYe-Chang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2024
@XiShanYongYe-Chang
Copy link
Member

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2024
@karmada-bot karmada-bot merged commit 23bcebc into karmada-io:master Oct 12, 2024
12 checks passed
@mohamedawnallah
Copy link
Contributor Author

Thanks, @XiShanYongYe-Chang, for merging it! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XiShanYongYe-Chang XiShanYongYe-Chang XiShanYongYe-Chang left review comments

@lonelyCZ lonelyCZ Awaiting requested review from lonelyCZ

@Poor12 Poor12 Awaiting requested review from Poor12

@chaosi-zju chaosi-zju Awaiting requested review from chaosi-zju

Assignees

@XiShanYongYe-Chang XiShanYongYe-Chang

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mohamedawnallah @codecov-commenter @XiShanYongYe-Chang @karmada-bot