Uncomment odo project list json test #3546
Conversation
amitkrout
force-pushed
the
odoProjectListTest
branch
from
c62d778
to
c2737e0
Compare
amitkrout
force-pushed
the
odoProjectListTest
branch
from
c2737e0
to
729f5e9
Compare
| Expect(err).Should(BeNil()) | ||
| partOfProjectListJSON, err := helper.Unindented(`{"kind":"Project","apiVersion":"odo.dev/v1alpha1","metadata":{"name":"` + project + `","namespace":"` + project + `","creationTimestamp":null},"spec":{},"status":{"active":true}}`) | ||
| Expect(err).Should(BeNil()) | ||
| Expect(listOutputJSON).To(ContainSubstring(partOfProjectListJSON)) |
There was a problem hiding this comment.
It wold be better to use MatchJSON instead of ContainSubstring
There was a problem hiding this comment.
@kadel yes, this a better option but it won't work the test scenario we are verifying. We are not matching the whole json here, some part of the json is being matched with the original output because projectListJSON := helper.CmdShouldPass("odo", "project", "list", "-o", "json") returns a huge output in CI and in locally the size depends on how many projects are there in that cluster when you ran the test script.
There was a problem hiding this comment.
Agreed, match json might not work as we are comparing a substring not the whole json. But then my question is why not? Why not compare the whole json output using MatchJSON seems more robust to me?
There was a problem hiding this comment.
But then my question is why not? Why not compare the whole json output using MatchJSON seems more robust to me?
@girishramnani json length is not the only concern, the major concern is the json length is dynamic. For example namespace myproject and ci-operator-hub-project are two default project we get when we run our CI test. So may be down the line we can add more namespace through the configure script. This can lead to change in json length.
The worst part we will face when we run the test script locally. For example you have a cluster and you running your test as kebeadmin then helper.CmdShouldPass("odo", "project", "list", "-o", "json") will list out almost 30 project details. It is weird to add a huge json list to match in test script.
There was a problem hiding this comment.
So to over come this i am just matching the key portion of the json list
|
/lgtm |
openshift-ci-robot
added
the
lgtm
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mohammedzee1000 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 |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Codecov Report
@@ Coverage Diff @@
## master #3546 +/- ##
=======================================
Coverage 46.27% 46.27%
=======================================
Files 112 112
Lines 11386 11386
=======================================
Hits 5269 5269
Misses 5608 5608
Partials 509 509 Continue to review full report at Codecov.
|
What type of PR is this?
/kind cleanup
What does does this PR do / why we need it:
Uncomment odo project list json test
Which issue(s) this PR fixes:
Fixes #1708
How to test changes / Special notes to the reviewer:
make test-cmd-project