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

Update to use Ginkgo BDD testing framework to align with the exist test style #487

Merged

Conversation

xiaoyu74
Copy link
Contributor

What type of PR is this?

(unit-test)

What this PR does / Why we need it?

Updated to use Ginkgo BDD testing framework for my healthcheck subcommand to align with the current test style

Which Jira/Github issue(s) does this PR fix?

Resolves #

Special notes for your reviewer

  • The main logic of the merged healthcheck subcommand is not change but just refactored the test cases using the Ginkgo BDD framework
  • Changed the struct name (e.g below) based on the naming convention
// Default implementations
type DefaultNetworkInterfaceImpl struct{}

type DefaultHTTPClientImpl struct {
        Client *http.Client
}
  • CheckBackplaneAPIConnectivity function is not covered by the test case since it's just call the existing CheckAPIConnection() which defined in the existing config.GetBackplaneConfiguration.

@openshift-ci openshift-ci bot requested review from bmeng and mjlshen July 21, 2024 21:51
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 21, 2024
@xiaoyu74 xiaoyu74 changed the title Updated to use Ginkgo BDD testing framework to align with the exist style Update to use Ginkgo BDD testing framework to align with the exist test style Jul 21, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2024

Codecov Report

Attention: Patch coverage is 31.25000% with 33 lines in your changes missing coverage. Please review.

Project coverage is 44.65%. Comparing base (74599c8) to head (5125f75).
Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #487      +/-   ##
==========================================
- Coverage   45.78%   44.65%   -1.13%     
==========================================
  Files          82       82              
  Lines        6199     6427     +228     
==========================================
+ Hits         2838     2870      +32     
- Misses       3011     3203     +192     
- Partials      350      354       +4     
Files Coverage Δ
pkg/healthcheck/check_proxy.go 72.34% <100.00%> (+35.75%) ⬆️
pkg/healthcheck/check_vpn.go 72.97% <100.00%> (ø)
pkg/healthcheck/connectivity_checks.go 8.43% <8.33%> (-2.24%) ⬇️

... and 5 files with indirect coverage changes

@samanthajayasinghe
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2024
Copy link
Contributor

openshift-ci bot commented Jul 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: samanthajayasinghe, xiaoyu74

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:
  • OWNERS [samanthajayasinghe,xiaoyu74]

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

Copy link
Contributor

openshift-ci bot commented Jul 24, 2024

@xiaoyu74: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 8ecb2fe into openshift:main Jul 24, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants