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

check if namespace is created instead of project #6108

Merged
merged 2 commits into from
Sep 16, 2022
Merged

check if namespace is created instead of project #6108

merged 2 commits into from
Sep 16, 2022

Conversation

anandrkskd
Copy link
Contributor

@anandrkskd anandrkskd commented Sep 9, 2022 •
edited by feloy
Loading

Signed-off-by: anandrkskd anandrkskd@gmail.com

What type of PR is this:

/kind tests

What does this PR do / why we need it:
Which issue(s) this PR fixes:
Fixes #

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@netlify
Copy link

netlify bot commented Sep 9, 2022 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit edb93a3
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6324794e28cbe7000895d336

@odo-robot
Copy link

odo-robot bot commented Sep 9, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Sep 9, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Sep 9, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Sep 9, 2022 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Sep 9, 2022 •
edited
Loading

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

valaparthvi
valaparthvi reviewed Sep 9, 2022
View reviewed changes
Copy link
Contributor

@valaparthvi valaparthvi left a comment

Choose a reason for hiding this comment

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

Why do we need this change? Isn't HasNamespaceProject enough?

tests/helper/helper_oc.go Outdated
@@ -582,6 +582,12 @@ func (oc OcRunner) HasNamespaceProject(name string) bool {
return strings.Contains(out, name)
}

func (oc OcRunner) HasNamespac(name string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func (oc OcRunner) HasNamespac(name string) bool {
func (oc OcRunner) HasNamespace(name string) bool {

@anandrkskd
Copy link
Contributor Author

anandrkskd commented Sep 9, 2022 •
edited
Loading

Why do we need this change? Isn't HasNamespaceProject enough?

We are still experiencing test flakes for these test cases (flake chart), even if we are checking for project. @feloy suggested it would be better to check if the namespace is present instead of project. This PR implements the same.

@anandrkskd anandrkskd force-pushed the test-failure branch 2 times, most recently from f9fc5ab to 86fa651 Compare September 13, 2022 10:07
kadel
kadel reviewed Sep 13, 2022
View reviewed changes
tests/helper/helper_oc.go Outdated Show resolved Hide resolved
feloy
feloy suggested changes Sep 13, 2022
View reviewed changes
tests/helper/helper_oc.go Outdated Show resolved Hide resolved
@anandrkskd anandrkskd force-pushed the test-failure branch 2 times, most recently from 3c12fa6 to e394c3b Compare September 14, 2022 05:58
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 14, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 14, 2022
@feloy
Copy link
Contributor

feloy commented Sep 15, 2022

/approve

@openshift-ci
Copy link

openshift-ci bot commented Sep 15, 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 Sep 15, 2022
dharmit
dharmit suggested changes Sep 15, 2022
View reviewed changes
tests/helper/helper_cli.go Show resolved Hide resolved
@dharmit
Copy link
Member

dharmit commented Sep 15, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 15, 2022
@rm3l rm3l closed this Sep 15, 2022
@rm3l rm3l reopened this Sep 15, 2022
@rm3l
Copy link
Member

rm3l commented Sep 16, 2022

@anandrkskd Can you please rebase your branch onto main?

I have the feeling that the potential issue (about Windows tests) I mentioned yesterday during standup is occurring here too. When looking at the logs, I can see that this test (should expose all endpoints on localhost regardless of exposure) is not passing:

+ [FAILED] [240.286 seconds]
odo dev command tests
C:/Users/Administrator.ANSIBLE-TEST-VS/2011/tests/integration/cmd_dev_test.go:31
  port-forwarding for the component
  C:/Users/Administrator.ANSIBLE-TEST-VS/2011/tests/integration/cmd_dev_test.go:471
    when devfile has multiple endpoints
    C:/Users/Administrator.ANSIBLE-TEST-VS/2011/tests/integration/cmd_dev_test.go:548
      when running odo dev
      C:/Users/Administrator.ANSIBLE-TEST-VS/2011/tests/integration/cmd_dev_test.go:555
        [It] should expose all endpoints on localhost regardless of exposure
        C:/Users/Administrator.ANSIBLE-TEST-VS/2011/tests/integration/cmd_dev_test.go:573

The problem is that this test was introduced recently in 16c6d15, but your branch does not contain this commit yet; so I'm confused ¯\_(ツ)_/¯

❯ git remote add anandrkskd git@github.com:anandrkskd/odo.git
❯ git fetch anandrkskd
❯ git branch -r --contains=16c6d1589f364ac81b679a48c9d86af6f76601b5
  origin/6f77dependabot/npm_and_yarn/docs/website/mdx-js/react-2.1.3
  origin/HEAD -> origin/main
  origin/fix-links-to-devfile.io
  origin/main
  upstream/dependabot/npm_and_yarn/docs/website/mdx-js/react-2.1.3
  upstream/main

@rm3l rm3l mentioned this pull request Sep 16, 2022
Merged
@anandrkskd
check if ns/project present
23c93b9 
Signed-off-by: anandrkskd <anandrkskd@gmail.com>
@anandrkskd
adding comment
edb93a3 
Signed-off-by: anandrkskd <anandrkskd@gmail.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 16, 2022
@openshift-ci
Copy link

openshift-ci bot commented Sep 16, 2022

New changes are detected. LGTM label has been removed.

@sonarcloud
Copy link

sonarcloud bot commented Sep 16, 2022

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.6% 0.6% Duplication

@rm3l rm3l added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 16, 2022
@rm3l
Copy link
Member

rm3l commented Sep 16, 2022

/test v4.11-integration-e2e

@anandrkskd
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit c3eced4 into redhat-developer:main Sep 16, 2022
@rm3l rm3l mentioned this pull request Sep 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valaparthvi valaparthvi valaparthvi left review comments

@feloy feloy feloy approved these changes

@kadel kadel kadel left review comments

@dharmit dharmit dharmit requested changes

Assignees

@dharmit dharmit

@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.
7 participants
@anandrkskd @feloy @dharmit @rm3l @kadel @valaparthvi @openshift-merge-robot