[RHTAPBUGS-418]multiple components with context path #346
Conversation
stuartwdouglas
force-pushed
the
component-test
branch
from
f4f4561
to
4fa3f96
Compare
stuartwdouglas
changed the title
| @@ -851,7 +851,28 @@ func DownloadDevfileAndDockerfile(url string) ([]byte, string, []byte, string) { | |||
| // Map 3 returns a context to the Dockerfile uri or a matched DockerfileURL from the devfile registry if no Dockerfile/Containerfile is present in the context | |||
| // Map 4 returns a context to the list of ports that were detected by alizer in the source code, at that given context | |||
| func ScanRepo(log logr.Logger, a Alizer, localpath string, devfileRegistryURL string, source appstudiov1alpha1.GitSource) (map[string][]byte, map[string]string, map[string]string, map[string][]int, error) { | |||
There was a problem hiding this comment.
I am not sure if the original method could still be used anywhere else, so I added a new one instead
There was a problem hiding this comment.
The original method should be fine to use.
There was a problem hiding this comment.
I have updated the PR
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #346 +/- ##
==========================================
- Coverage 84.61% 83.81% -0.81%
==========================================
Files 27 27
Lines 3530 4152 +622
==========================================
+ Hits 2987 3480 +493
- Misses 402 504 +102
- Partials 141 168 +27
☔ View full report in Codecov by Sentry. |
stuartwdouglas
changed the title
stuartwdouglas
force-pushed
the
component-test
branch
from
bebb2a0
to
8d6ce86
Compare
Test for RHTAPBUGS-418
stuartwdouglas
force-pushed
the
component-test
branch
2 times, most recently
from
8f22e40
to
5940a4f
Compare
|
Does this need any more action on my part? |
| @@ -284,7 +284,7 @@ func (r *ComponentDetectionQueryReconciler) Reconcile(ctx context.Context, req c | |||
| if isMultiComponent { | |||
| log.Info(fmt.Sprintf("Since this is a multi-component, attempt will be made to read only level 1 dir for devfiles... %v", req.NamespacedName)) | |||
|
|
|||
| devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, err = devfile.ScanRepo(log, r.AlizerClient, componentPath, r.DevfileRegistryURL, source) | |||
| devfilesMap, devfilesURLMap, dockerfileContextMap, componentPortsMap, err = devfile.ScanRepo(log, r.AlizerClient, source.Context, componentPath, r.DevfileRegistryURL, source) | |||
There was a problem hiding this comment.
the source is already been passed in as a arg. no need an extra arg for source.Context
stuartwdouglas
force-pushed
the
component-test
branch
4 times, most recently
from
9896d84
to
b40acdb
Compare
stuartwdouglas
force-pushed
the
component-test
branch
from
b40acdb
to
eeef423
Compare
|
Fixed |
pkg/devfile/detect.go
Outdated
| } else if !strings.HasSuffix(originalContext, "/") { | ||
| originalContext += "/" | ||
| } | ||
| } |
There was a problem hiding this comment.
do not need this code change. the context will never be empty. as it's already set in
application-service/controllers/componentdetectionquery_controller.go
Lines 147 to 149 in eeef423
and with the path.join, you do not need to append / at the end
yangcao77
changed the title
Fixes RHTAPBUGS-418
stuartwdouglas
force-pushed
the
component-test
branch
from
eeef423
to
b053120
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stuartwdouglas, 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 |
Test for RHTAPBUGS-418
What does this PR do?:
Test for an issue I saw in the UI where importing a project with multiple components and a context path set results in the context path going missing.
I could not get this test to run locally as all the detection tests are timing out for me.
Which issue(s)/story(ies) does this PR fixes:
https://issues.redhat.com/browse/RHTAPBUGS-418