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

Add more tests for Podman + write Component abstraction for integration tests #6427

Merged
merged 4 commits into from
Dec 20, 2022
Merged

Add more tests for Podman + write Component abstraction for integration tests #6427

merged 4 commits into from
Dec 20, 2022

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Dec 19, 2022 •
edited
Loading

What type of PR is this:

/kind tests

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes partially #6416

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@netlify
Copy link

netlify bot commented Dec 19, 2022 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 2015979
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63a0a762ceac82000903db1d

@feloy feloy requested review from valaparthvi and rm3l and removed request for valaparthvi and rm3l December 19, 2022 14:11
@feloy feloy requested review from rm3l and removed request for anandrkskd and rnapoles-rh December 19, 2022 14:11
@feloy feloy changed the title Add more tests for Podman + write Component abstraction for integration tests [WIP] Add more tests for Podman + write Component abstraction for integration tests Dec 19, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Dec 19, 2022
@odo-robot
Copy link

odo-robot bot commented Dec 19, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Dec 19, 2022 •
edited
Loading

NoCluster Tests on commit a5e8e85 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Dec 19, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Dec 19, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Dec 19, 2022 •
edited
Loading

Windows Tests (OCP) on commit a5e8e85 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Dec 19, 2022 •
edited
Loading

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

@feloy feloy force-pushed the tests-6416/more-podman-tests branch from 3cc4d55 to 2177bba Compare December 19, 2022 15:20
@feloy feloy removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Dec 20, 2022
@feloy feloy changed the title [WIP] Add more tests for Podman + write Component abstraction for integration tests Add more tests for Podman + write Component abstraction for integration tests Dec 20, 2022
@feloy feloy closed this Dec 20, 2022
@feloy feloy reopened this Dec 20, 2022
@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

rm3l
rm3l reviewed Dec 20, 2022
View reviewed changes
tests/helper/component_podman.go

func (o *PodmanComponent) ExpectIsNotDeployed() {
podName := fmt.Sprintf("%s-%s", o.name, o.app)
cmd := exec.Command("podman", "pod", "list", "--format", "{{.Name}}", "--noheading")
Copy link
Member

@rm3l rm3l Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Would it make sense to make the path to podman configurable (ideally using the PODMAN_CMD env var just like with the internal PodmanCli client)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. This is used during integration tests only, on a stable environment, I cannot see a reason for the moment to have to configure it

Copy link
Member

Choose a reason for hiding this comment

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

I agree from the environment perspective.
I thought it would make sense for an odo dev to make sure the same podman binary is used when running the actual odo commands in the tests and when performing the assertions here..
But okay, that's something that can be optimized later on.
/lgtm

@feloy feloy requested a review from rm3l December 20, 2022 09:53
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Dec 20, 2022
@rm3l
Copy link
Member

rm3l commented Dec 20, 2022 

Container test exited with code 1, reason Error
---
aws-cli/2.4.15 Python/3.8.8 Linux/4.18.0-372.36.1.el8_6.x86_64 exe/x86_64.rhel.7 prompt/off
Gathering console logs for i-01243d8acedeee344
An error occurred (InvalidInstanceID.NotFound) when calling the GetConsoleOutput operation: The instance ID 'i-01243d8acedeee344' does not exist

Looks like an infra error on Prow. And all tests are passing on IBM Cloud.

/override ci/prow/v4.11-integration-e2e

@openshift-ci
Copy link

openshift-ci bot commented Dec 20, 2022

@rm3l: Overrode contexts on behalf of rm3l: ci/prow/v4.11-integration-e2e

In response to this:

Container test exited with code 1, reason Error
---
aws-cli/2.4.15 Python/3.8.8 Linux/4.18.0-372.36.1.el8_6.x86_64 exe/x86_64.rhel.7 prompt/off
Gathering console logs for i-01243d8acedeee344
An error occurred (InvalidInstanceID.NotFound) when calling the GetConsoleOutput operation: The instance ID 'i-01243d8acedeee344' does not exist

Looks like an infra error on Prow. And all tests are passing on IBM Cloud.

/override ci/prow/v4.11-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.

@feloy
Copy link
Contributor Author

feloy commented Dec 20, 2022

/override ci/prow/v4.11-integration-e2e
Infra error
Tests pass on IBM Cloud

@openshift-ci
Copy link

openshift-ci bot commented Dec 20, 2022

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

In response to this:

/override ci/prow/v4.11-integration-e2e
Infra error
Tests pass on IBM Cloud

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-merge-robot openshift-merge-robot merged commit 4aad7ed into redhat-developer:main Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rm3l rm3l rm3l approved these changes

@valaparthvi valaparthvi Awaiting requested review from valaparthvi

Assignees

@rm3l rm3l

Labels
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
@feloy @rm3l @openshift-merge-robot