[flaky test] Move DetectName to method in alizer package #6093
Conversation
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
feloy
changed the title
|
Did you figure out why this test passes when run individually but fails in a batch? |
Not exactly, but the unit test were working on the real filesystem and calling the alizer library, because the DetectName were not mocked. This is what we want to avoid for unit tests |
openshift-ci
bot
added
the
lgtm
| @@ -57,7 +57,8 @@ func GatherName(contextDir string, devfileObj *parser.DevfileObj) (string, error | |||
| // Use Alizer if Devfile has no (optional) metadata.name field. | |||
| // We need to pass in the Devfile base directory (not the path to the devfile.yaml). | |||
| // Name returned by alizer.DetectName is expected to be already sanitized. | |||
| return alizer.DetectName(filepath.Dir(devfileObj.Ctx.GetAbsPath())) | |||
| alizerClient := alizer.Alizer{} // TODO(feloy) fix with DI | |||
There was a problem hiding this comment.
I am not sure how this will be fixed with DI given that GatherName is called from context pkg. A simpler solution that I can think of, is to let DetectName be a function, and let DetectName method call the function, that way it can still be mocked without having to fix this with DI.
There was a problem hiding this comment.
I prefer not to, or future developers will be tempted to call the function, even if the method could be called
|
/approve |
openshift-ci
bot
added
the
approved
|
Re-run with new names |
|
Kudos, SonarCloud Quality Gate passed!
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: valaparthvi 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 |
|
/override OpenShift-Integration-tests/OpenShift-Integration-tests |
|
@feloy: Overrode contexts on behalf of feloy: OpenShift-Integration-tests/OpenShift-Integration-tests In response to this:
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/test-infra repository. |
|
/override ci/prow/v4.10-integration-e2e |
|
@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.10-integration-e2e In response to this:
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/test-infra repository. |
What type of PR is this:
/kind bug
What does this PR do / why we need it:
Move the DetectName function to a method in the
pkg/alizermethod, to be able to mock it frompkg/init/backendpackageWhich issue(s) this PR fixes:
Fixes #6092
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer: