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.

Sign up for GitHub

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

Jump to bottom

Fix potential issue with image.TestSelectBackend unit tests when PODMAN_CMD env var is set #5614

Merged
merged 1 commit into from
Apr 1, 2022
Merged

Fix potential issue with image.TestSelectBackend unit tests when PODMAN_CMD env var is set #5614

merged 1 commit into from
Apr 1, 2022

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Mar 31, 2022 •
edited
Loading

What type of PR is this:
/kind tests

Which issue(s) this PR fixes:
For some valid reasons, we could want odo to use Docker as backend when building images.
One way of doing so is to set the PODMAN_CMD system environment variable to a command we know does not exist.
This is what I did a few days ago on my system.
Now the image unit tests will not pass, because they rely on the system environment variables.

~/w/p/odo on  main [$] via 🐹 v1.16.15 on ☁️   
❯ PODMAN_CMD=a-command-not-found-should-make-odo-use-docker go test -race github.com/redhat-developer/odo/pkg/devfile/image

↪ Building & Pushing Container: a name

↪ Building & Pushing Container: a name

↪ Building & Pushing Container: a name

↪ Building & Pushing Container: a name
--- FAIL: TestSelectBackend (0.00s)
    --- FAIL: TestSelectBackend/all_backends_are_present (0.00s)
        image_test.go:172: all backends are present: Error backend wanted podman, got a-command-not-found-should-make-odo-use-docker
    --- FAIL: TestSelectBackend/only_podman_is_present (0.00s)
        image_test.go:168: only podman is present: Error result wanted false, got true
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1e9caf3]

goroutine 69 [running]:
testing.tRunner.func1.2(0x1fb62c0, 0x2e503b0)
        /home/asoro/.asdf/installs/golang/1.16.15/go/src/testing/testing.go:1153 +0x49f
testing.tRunner.func1(0xc0005b0180)
        /home/asoro/.asdf/installs/golang/1.16.15/go/src/testing/testing.go:1156 +0x695
panic(0x1fb62c0, 0x2e503b0)
        /home/asoro/.asdf/installs/golang/1.16.15/go/src/runtime/panic.go:971 +0x499
github.com/redhat-developer/odo/pkg/devfile/image.TestSelectBackend.func5(0xc0005b0180)
        /home/asoro/work/projects/odo/pkg/devfile/image/image_test.go:171 +0x173
testing.tRunner(0xc0005b0180, 0xc000692960)
        /home/asoro/.asdf/installs/golang/1.16.15/go/src/testing/testing.go:1203 +0x203
created by testing.(*T).Run
        /home/asoro/.asdf/installs/golang/1.16.15/go/src/testing/testing.go:1248 +0x5d8
FAIL    github.com/redhat-developer/odo/pkg/devfile/image       0.047s
FAIL

A workaround is to unset the PODMAN_CMD system environment variable prior to running the unit tests, but I found it cleaner to adapt the unit tests, so they run in a controlled environment.

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:
The unit tests should pass regardless of the value of PODMAN_CMD:

~/w/p/odo on  fix_image_unit_test_if_PODMAN_CMD_envvar_set [$] via 🐹 v1.16.15 on ☁️   
❯ PODMAN_CMD=a-command-not-found-should-make-odo-use-docker go test -race github.com/redhat-developer/odo/pkg/devfile/image
ok      github.com/redhat-developer/odo/pkg/devfile/image

@rm3l
Fix potential issue with image unit tests when PODMAN_CMD env var…
6f04813 
… is set

For some valid reasons, we could want `odo` to use Docker as backend
when building images.
One way of doing so is to set the `PODMAN_CMD` system environment variable to a command we know does not exist.
In such cases, the `image` unit tests will not pass, because they rely on the system environment variables.
@rm3l rm3l requested a review from feloy March 31, 2022 11:38
@netlify
Copy link

netlify bot commented Mar 31, 2022 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 6f04813
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/624592cbef74c000092d76c7

@openshift-ci openshift-ci bot requested a review from mohammedzee1000 March 31, 2022 11:39
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@odo-robot
Copy link

odo-robot bot commented Mar 31, 2022

Unit Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 31, 2022

Kubernetes Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 31, 2022

OpenShift Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 31, 2022

Validate Tests on commit finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor

feloy commented Apr 1, 2022

/lgtm
/approve
/override ci/prow/v4.10-integration-e2e

@openshift-ci
Copy link

openshift-ci bot commented Apr 1, 2022

@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.10-integration-e2e

In response to this:

/lgtm
/approve
/override ci/prow/v4.10-integration-e2e

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.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 1, 2022
@openshift-ci
Copy link

openshift-ci bot commented Apr 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feloy

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Apr 1, 2022
@openshift-merge-robot openshift-merge-robot merged commit 851b2f3 into redhat-developer:main Apr 1, 2022
@rm3l rm3l deleted the fix_image_unit_test_if_PODMAN_CMD_envvar_set branch April 1, 2022 07:03
cdrage pushed a commit to cdrage/odo that referenced this pull request Aug 31, 2022
@rm3l @cdrage
Fix potential issue with image unit tests when PODMAN_CMD env var…
0600d67 
… is set (redhat-developer#5614)

For some valid reasons, we could want `odo` to use Docker as backend
when building images.
One way of doing so is to set the `PODMAN_CMD` system environment variable to a command we know does not exist.
In such cases, the `image` unit tests will not pass, because they rely on the system environment variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feloy feloy Awaiting requested review from feloy

@mohammedzee1000 mohammedzee1000 Awaiting requested review from mohammedzee1000

Assignees

@feloy feloy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rm3l @feloy @openshift-merge-robot