[DEVHAS-491] Mock SPI Client & Devfile Utils Client for Pvt Comp Controller Tests #421
Conversation
|
Once devfile/library#191 is merged, I should be pull in the latest commit and teh tests should pass. |
Signed-off-by: Maysun J Faisal <[email protected]>
Signed-off-by: Maysun J Faisal <[email protected]>
maysunfaisal
force-pushed
the
DEVHAS-491
branch
from
353db29 to
b321929
Compare
Signed-off-by: Maysun J Faisal <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #421 +/- ##
==========================================
- Coverage 81.34% 81.26% -0.08%
==========================================
Files 32 32
Lines 4787 4789 +2
==========================================
- Hits 3894 3892 -2
- Misses 702 705 +3
- Partials 191 192 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Maysun J Faisal <[email protected]>
| @@ -401,7 +403,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( | |||
| if devfileLocation != "" { | |||
| // Parse the Component Devfile | |||
| log.Info(fmt.Sprintf("Parsing Devfile from the Devfile location %s... %v", devfileLocation, req.NamespacedName)) | |||
| compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{URL: devfileLocation, Token: gitToken}) | |||
| compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{URL: devfileLocation, Token: gitToken, DevfileUtilsClient: r.DevfileUtilsClient}) | |||
There was a problem hiding this comment.
I saw in component controller, it calls multiple devfile parser. why only pass in DevfileUtilsClient in this place? because the test case only covers here?
also should this be tested in CDQ module as well?
There was a problem hiding this comment.
Good Question. Because:
- This is the only place in component controller which uses URL
devfileParser.ParserArgs{URL: devfileLocation ..., other invocations use&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile) .... - https://issues.redhat.com/browse/DEVHAS-492 deals with testing private links in the devfile, so when a Devfile is from a data src and has private links for URI for example, then we can provide
DevfileUtilsClientin these calls - Also, devfile/library does a default assignment if its nil, https://github.com/devfile/library/blob/main/pkg/devfile/parser/parse.go#L173-L175. So even if we dont provide argument with the call here, it will always use the real implementation. When we are working on https://issues.redhat.com/browse/DEVHAS-492, we can change it to allow for mock tests.
There was a problem hiding this comment.
I found one potential call in CDQ which may have been missed, let me look into that and open a spike investigation issue, since its out of the scope of this.
Otherwise the remaining CDQ calls are with public registry which we dont need token. Also application controller doesnt need a token because we construct the devfile for application CR.
| Source: appstudiov1alpha1.ComponentSource{ | ||
| ComponentSourceUnion: appstudiov1alpha1.ComponentSourceUnion{ | ||
| GitSource: &appstudiov1alpha1.GitSource{ | ||
| URL: SampleRepoLink, |
There was a problem hiding this comment.
the test says for private repo, is this samplerepolink for public repo testing tho?
There was a problem hiding this comment.
Actually it doesn't matter what repo we provide here, we are testing the token "valid-token" which returns a mock devfile from See https://github.com/devfile/library/blob/main/pkg/util/mock.go#L250. But I changed this line to a private repo to avoid confusion and added a comment to the mock file. 👍
Signed-off-by: Maysun J Faisal <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maysunfaisal, yangcao77 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 |
What does this PR do?:
Which issue(s)/story(ies) does this PR fixes:
https://issues.redhat.com/browse/DEVHAS-491
PR acceptance criteria:
Unit/Functional tests
Documentation
Client Impact
How to test changes / Special notes to the reviewer:
Tests should pass