Fixes project wait flag #2221

mik-dass · 2019-10-01T14:39:16Z

How to test:
Check if the odo project create <project-name> -w works and it waits properly for the project to be ready.

amitkrout · 2019-10-03T11:31:15Z

tests/integration/cmd_project_test.go

 			listOutput := helper.CmdShouldPass("odo", "project", "list")
 			Expect(listOutput).To(ContainSubstring(project))

 			// project deletion doesn't happen immediately, so we test subset of the string
 			listOutputJson := helper.CmdShouldPass("odo", "project", "list", "-o", "json")
-			Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":false}}`))
+			Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}`))


In parallel run validating the projects status "status":{"active":true}} or "status":{"active":true}} could lead to a flaky behavior. Instead of matching the full json can you match some major key value pair.

For now see how this is being done in one of our test cases - https://github.com/openshift/odo/blob/master/tests/integration/component.go#L197

amitkrout · 2019-10-03T11:31:44Z

tests/integration/cmd_project_test.go

@@ -41,13 +41,12 @@ var _ = Describe("odo project command tests", func() {
 	Context("when running project command app parameter in directory that doesn't contain .odo config directory", func() {
 		It("should successfully execute list along with machine readable output", func() {
 			helper.CmdRunner("odo", "project", "list")
-			time.Sleep(1 * time.Second)


amitkrout · 2019-10-03T11:38:03Z

tests/integration/cmd_project_test.go

 			helper.CmdRunner("odo", "project", "list")
-			time.Sleep(1 * time.Second)


Also remove helper.CmdRunner("odo", "project", "list"). In the next line it is being duplicated.

mik-dass · 2019-10-03T12:16:47Z

✗  Waiting for component to start [4m]
[odo]  ✗  waited 4m0s but couldn't find running pod matching selector: 'deploymentconfig=mycomponent-myapp'

/retest

mik-dass · 2019-10-03T12:17:56Z

@amitkrout Done

amitkrout · 2019-10-03T12:32:05Z

/approve

cdrage

Failing test / you need to modify your current test to make sure that it will pass..

cdrage · 2019-10-03T12:40:51Z

tests/integration/cmd_project_test.go

 			listOutput := helper.CmdShouldPass("odo", "project", "list")
 			Expect(listOutput).To(ContainSubstring(project))

 			// project deletion doesn't happen immediately, so we test subset of the string
 			listOutputJson := helper.CmdShouldPass("odo", "project", "list", "-o", "json")
-			Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":false}}`))
+			Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},`))


Shouldn't this be status: active? Why was it removed? This is obvious that the test is failing.. In this test scenario, you can modify the beforeEach to add -w to wait for the project to come up.

Shouldn't this be status: active? Why was it removed? This is obvious that the test is failing..

@cdrage #2221 (comment)

Think a case where you have more scenarios are running for example make test-integration then in two node you cannot be sure that the current scenario project is active throughout the scenario. May be in the same time some different project scenario is created and become active.

In this test scenario, you can modify the beforeEach to add -w to wait for the project to come up

In the test script we have already made this behavior as default to avoid flakes. see - https://github.com/openshift/odo/blob/master/tests/helper/odo_utils.go#L79. The failure was happening due to the step CmdRunner(...) which does not wait for command output.

As @amitkrout pointed due to parallel tests we can't be sure of the activeness of the test throughout the scenario so I removed the status. @cdrage @kadel WDYT?

Ah, but that defeats the whole purpose of the test though, to make sure that it's the active project. Any way to test that individually without parallelism?

yup, there is an option of sequential run. In a long run it would be hard to even manage get and set command in parallel run. @mik-dass Please do the change to run it on single node test.

For reference check - https://github.com/openshift/odo/tree/master/tests/integration/loginlogout.

openshift-ci-robot · 2019-10-04T10:23:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amitkrout

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:

~~OWNERS~~ [amitkrout]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mik-dass · 2019-10-04T10:25:17Z

@cdrage @amitkrout Done please have a look

amitkrout · 2019-10-04T10:34:44Z

@cdrage @amitkrout Done please have a look

Looks good to me.

mik-dass · 2019-10-04T11:57:12Z

[oc] Error in configuration: 
[oc] * context was not found for specified context: yfkoarskib/api-ci-op-r1xy5kvm-1b2f4-origin-ci-int-aws-dev-rhcloud-com:6443/developer
[oc] * cluster has no server defined

/retest

cdrage

Thanks for the tests. These work way better.

/lgtm

cdrage · 2019-10-04T12:59:51Z

tests/integration/project/cmd_project_test.go

 			listOutput := helper.CmdShouldPass("odo", "project", "list")
 			Expect(listOutput).To(ContainSubstring(project))

 			// project deletion doesn't happen immediately, so we test subset of the string
 			listOutputJson := helper.CmdShouldPass("odo", "project", "list", "-o", "json")
-			Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":false}}`))
+			Expect(listOutputJson).To(ContainSubstring(`{"kind":"Project","apiVersion":"odo.openshift.io/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}`))


Thanks! 👍 way better.

openshift-bot · 2019-10-04T15:12:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-10-04T17:22:03Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the size/XS label Oct 1, 2019

openshift-ci-robot requested review from amitkrout and girishramnani October 1, 2019 14:40

mik-dass requested a review from kadel October 1, 2019 14:44

mik-dass changed the title ~~Fixes project wait flag~~ [WIP] Fixes project wait flag Oct 1, 2019

openshift-ci-robot 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 Oct 1, 2019

mik-dass changed the title ~~[WIP] Fixes project wait flag~~ Fixes project wait flag Oct 3, 2019

openshift-ci-robot 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 Oct 3, 2019

mik-dass force-pushed the fix_proj_wait branch from 8ed2529 to acb1f4b Compare October 3, 2019 09:19

openshift-ci-robot added size/S and removed size/XS labels Oct 3, 2019

Fixes project wait flag

2c0a3c5

mik-dass force-pushed the fix_proj_wait branch from acb1f4b to 20144f8 Compare October 3, 2019 09:20

amitkrout suggested changes Oct 3, 2019

View reviewed changes

amitkrout reviewed Oct 3, 2019

View reviewed changes

amitkrout suggested changes Oct 3, 2019

View reviewed changes

mik-dass force-pushed the fix_proj_wait branch from 20144f8 to 0969c09 Compare October 3, 2019 12:09

openshift-ci-robot added size/M and removed size/S labels Oct 3, 2019

mik-dass force-pushed the fix_proj_wait branch from 0969c09 to ec18008 Compare October 3, 2019 12:28

openshift-ci-robot added size/S and removed size/M labels Oct 3, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Oct 3, 2019

cdrage suggested changes Oct 3, 2019

View reviewed changes

mik-dass force-pushed the fix_proj_wait branch from ec18008 to b942e32 Compare October 4, 2019 10:23

openshift-ci-robot added size/M and removed size/S labels Oct 4, 2019

Fix project create unit test in occlient

d473b60

mik-dass force-pushed the fix_proj_wait branch from b942e32 to d473b60 Compare October 4, 2019 10:23

openshift-ci-robot added size/S and removed size/M labels Oct 4, 2019

cdrage approved these changes Oct 4, 2019

View reviewed changes

openshift-ci-robot assigned cdrage Oct 4, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Oct 4, 2019

openshift-merge-robot merged commit f569be9 into redhat-developer:master Oct 4, 2019

mik-dass deleted the fix_proj_wait branch October 17, 2019 08:46

mik-dass mentioned this pull request Oct 29, 2019

Project create doesn't wait properly #2117

Closed

amitkrout mentioned this pull request Nov 5, 2019

Commenting project command test flake spec #2356

Closed

rm3l added the estimated-size/S (5-10) Rough sizing for Epics. Less then one sprint of work for one person label Jun 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes project wait flag #2221

Fixes project wait flag #2221

mik-dass commented Oct 1, 2019

amitkrout Oct 3, 2019

amitkrout Oct 3, 2019

amitkrout Oct 3, 2019

mik-dass commented Oct 3, 2019

mik-dass commented Oct 3, 2019

amitkrout commented Oct 3, 2019

cdrage left a comment

cdrage Oct 3, 2019

amitkrout Oct 3, 2019

mik-dass Oct 3, 2019

cdrage Oct 3, 2019 •

edited

amitkrout Oct 3, 2019

openshift-ci-robot commented Oct 4, 2019

mik-dass commented Oct 4, 2019

amitkrout commented Oct 4, 2019

mik-dass commented Oct 4, 2019

cdrage left a comment

cdrage Oct 4, 2019

openshift-bot commented Oct 4, 2019

openshift-bot commented Oct 4, 2019

		helper.CmdRunner("odo", "project", "list")
		time.Sleep(1 * time.Second)

Fixes project wait flag #2221

Fixes project wait flag #2221

Conversation

mik-dass commented Oct 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mik-dass commented Oct 3, 2019

mik-dass commented Oct 3, 2019

amitkrout commented Oct 3, 2019

cdrage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdrage Oct 3, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 4, 2019

mik-dass commented Oct 4, 2019

amitkrout commented Oct 4, 2019

mik-dass commented Oct 4, 2019

cdrage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-bot commented Oct 4, 2019

openshift-bot commented Oct 4, 2019

cdrage Oct 3, 2019 •

edited