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

Improve test coverage of backend pt2 #615

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

jopit
Copy link
Contributor

@jopit jopit commented Aug 17, 2023

Description:

  • Add unit tests for 'reconcileRepositoryCredentialStatus' in the backend component.

Link to JIRA Story (if applicable): https://issues.redhat.com/browse/GITOPSRVCE-696

Signed-off-by: John Pitman <jpitman@redhat.com>
@openshift-ci openshift-ci bot requested review from chetan-rns and jgwest August 17, 2023 18:58
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.07% 🎉

Comparison is base (4a6afaf) 56.04% compared to head (d32fb5d) 56.12%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
+ Coverage   56.04%   56.12%   +0.07%     
==========================================
  Files         100      100              
  Lines       17596    17596              
==========================================
+ Hits         9862     9876      +14     
+ Misses       6513     6497      -16     
- Partials     1221     1223       +2     

see 1 file with indirect coverage changes

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

@jgwest jgwest removed their request for review August 22, 2023 19:15
Copy link
Contributor

@chetan-rns chetan-rns left a comment

Choose a reason for hiding this comment

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

Looks great @jopit . I think it needs a rebase to sync with the latest main

/lgtm

Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @jopit, and thanks @chetan-rns for reviewing!

One final request: in reconcileRepositoryCredentialStatus, the following block isn't covered, but it should just be a matter of writing a quick test to call this function with an invalid objectMeta, and verify it doesn't explode:

		log.Info(fmt.Sprintf("could not find GitopsDeploymentRepositoryCredential %s in the cluster. Skipping reconciliation.", gitopsDeploymentRepositoryCredentialCR.GetName()))
		return

backend/eventloop/repocred_reconciler_test.go Outdated Show resolved Hide resolved
backend/eventloop/repocred_reconciler_test.go Outdated Show resolved Hide resolved
backend/eventloop/repocred_reconciler_test.go Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the lgtm label Aug 29, 2023
Signed-off-by: John Pitman <jpitman@redhat.com>
@jopit
Copy link
Contributor Author

jopit commented Sep 21, 2023

I've made all requested changes.

Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

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

Perfect, thanks again @jopit!

@openshift-ci openshift-ci bot added the lgtm label Sep 21, 2023
@jgwest jgwest merged commit 462a83d into redhat-appstudio:main Sep 21, 2023
12 checks passed
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chetan-rns, jgwest, jopit

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants