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.

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

Fix some flaky tests #5586

Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Mar 22, 2022

What type of PR is this:

/kind bug

What does this PR do / why we need it:

Which issue(s) this PR fixes:

odo delete command tests
/go/src/github.com/redhat-developer/odo/tests/integration/devfile/cmd_delete_test.go:13
  when a component is bootstrapped
  /go/src/github.com/redhat-developer/odo/tests/integration/devfile/cmd_delete_test.go:32
    when the component is deployed in DEV mode and dev mode stopped
    /go/src/github.com/redhat-developer/odo/tests/integration/devfile/cmd_delete_test.go:68
      when the component is deleted using its name and namespace from another directory
      /go/src/github.com/redhat-developer/odo/tests/integration/devfile/cmd_delete_test.go:76
        when odo delete command is run again with nothing deployed on the cluster
        /go/src/github.com/redhat-developer/odo/tests/integration/devfile/cmd_delete_test.go:94
          should output that there are no resources to be deleted [It]
          /go/src/github.com/redhat-developer/odo/tests/integration/devfile/cmd_delete_test.go:102
          Expected
              
              <string>: Searching resources to delete, please wait...
              This will delete "mynodejs" from the namespace "cmd-delete-test102goj".
               •  The component contains the following resources that will get deleted:
              	- PodMetrics: mynodejs-app-6d84cf9975-jw4pv
              The component "mynodejs" is successfully deleted from namespace "cmd-delete-test102goj"
              
          to contain substring
              <string>: No resource found for component "mynodejs" in namespace "cmd-delete-test102goj"

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 22, 2022
@netlify
Copy link

netlify bot commented Mar 22, 2022

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 3d3a8f5
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/623c9af10055cc0009b2bdaf

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2022

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

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2022

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

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2022

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

@feloy
Copy link
Contributor Author

feloy commented Mar 22, 2022

Kubernetes Tests on commit finished with errors. View logs: TXT HTML

@anandrkskd Could you please increase the number of nodes on this cluster?

[odo] I0322 18:47:39.453150   11036 events.go:53] Warning Event: Count: 5, Reason: FailedScheduling, Message: 0/3 nodes are available: 3 Insufficient memory.

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2022

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

@feloy feloy force-pushed the tests-fix-after-delete branch from a3d5dbd to 2e5240e Compare March 22, 2022 20:31
@feloy
Copy link
Contributor Author

feloy commented Mar 22, 2022

/test v4.10-integration-e2e
Infra error

1 similar comment
@feloy
Copy link
Contributor Author

feloy commented Mar 23, 2022

/test v4.10-integration-e2e
Infra error

@feloy feloy changed the title Eventually after delete component Fix some flaky tests Mar 23, 2022
@rm3l
Copy link
Member

rm3l commented Mar 23, 2022

Thanks! I had the exact same issue reported in another PR, and was wondering what could cause this,... until I saw this PR :)

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 23, 2022
@feloy feloy force-pushed the tests-fix-after-delete branch from 2e5240e to 3d3a8f5 Compare March 23, 2022 10:09
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 23, 2022
@valaparthvi
Copy link
Contributor

/retest-required

Comment on lines +102 to +105
Eventually(func() string {
stdOut = helper.Cmd("odo", "delete", "component", "--name", cmpName, "--namespace", commonVar.Project, "-f").ShouldPass().Out()
return stdOut
}, 60, 3).Should(ContainSubstring("No resource found for component %q in namespace %q", cmpName, commonVar.Project))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think instead of this, you can wait for Pods to not be present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I would really want to test that the delete component command displays that no resources found

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant, you wait for the pods to not be present, and then run the delete command to ensure it gives the right message. Why do you choose otherwise?

@feloy feloy force-pushed the tests-fix-after-delete branch from 3d3a8f5 to ac862b7 Compare March 24, 2022 16:23
Copy link
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

/lgtm

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

valaparthvi commented Mar 25, 2022

@feloy Why have IBM cloud tests not triggered yet?
I mean can you re-push so that they'll be triggered?

@valaparthvi
Copy link
Contributor

/approve
Tests pass locally with multiple runs.

@valaparthvi valaparthvi reopened this Mar 25, 2022
@openshift-ci
Copy link

openshift-ci bot commented Mar 25, 2022

[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 /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 Mar 25, 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

@feloy
Copy link
Contributor Author

feloy commented Mar 25, 2022

IBM Cloud OpenShift & Kubernetes tests pass

Exceeded limit MAX_RRSETS_BY_ZONE for zone: ...

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

@openshift-ci
Copy link

openshift-ci bot commented Mar 25, 2022

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

In response to this:

IBM Cloud OpenShift & Kubernetes tests pass

Exceeded limit MAX_RRSETS_BY_ZONE for zone: ...

/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-merge-robot openshift-merge-robot merged commit bad34a9 into redhat-developer:main Mar 25, 2022
cdrage pushed a commit to cdrage/odo that referenced this pull request Aug 31, 2022
* Eventually after delete component

* Fix for ps -ef error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Flake] odo dev test should run odo dev on initial namespace failing on periodic jobs
5 participants