Fix 'odo version' integration test

[zhengxiaomei123]

/kind bug

What versions of software are you using?

Operating System:
Red Hat Enterprise Linux 8.2 (Ootpa)
Output of odo version:
odo v1.2.2 (b815649)

How did you run odo exactly?

make test-generic

Actual behavior

/should show the version of odo major components including server login UR
should show the version of odo major components including server login URL [It]
go/src/github.com/openshift/odo/tests/integration/generic_test.go:267

Expected
    <bool>: false
to be true

/go/src/github.com/openshift/odo/tests/integration/generic_test.go:277

Expected behavior

The case passed

Any logs, error output, etc?

To test the command odo version include the server URL, There is regexp : Server:\s*https:\/\/(.+\.com|([0-9]+.){3}[0-9]+):[0-9]{4}
The regexp is not correct for all possible URL. For example:
Server: https://api.ocp43b.rhocp.boe:6443 is not match , Server: https://api.ocp-test.redhat.com:6443 is match.

[zhengxiaomei123]

What is the fact pattern for Server URL?

[zhengxiaomei123]

http(s)://localhost:6443
http(s)://api.example.test.com:6443
http(s)://api.ocp43b.rhocp.boe:6443
http(s)://127.0.0.1:8443

[metacosm]

Do we really need to check that the version conforms to a regexp? Isn't it enough to check that it returns something for the Server "field"?

[amitkrout]

Do we really need to check that the version conforms to a regexp? Isn't it enough to check that it returns something for the Server "field"?

@metacosm No, because the server output pattern deserves a validation to get precise field validation for integration test.

[metacosm]

@amitkrout care to explain in greater details, please?

[amitkrout]

@amitkrout care to explain in greater details, please?

@metacosm odo version details should not print a blank/weird server pattern. Same is applicable for Kubernetes version listed in odo version

Does it make sense ?

[metacosm]

@amitkrout sure but then wouldn’t it make more sense to retrieve the url of the cluster independently in the test and check that odo returns that instead of checking that it conforms to a regexp, which, while better than nothing, actually doesn’t check that odo returns the proper value?

[metacosm]

@amitkrout or retrieve the URL from the odo version output and check that you can use it to connect to an actual cluster?

[amitkrout]

@metacosm Our circumstance is like, as a prerequisite we need to login to the cluster first then we start executing the test spec for example odo version spec. So in our test scenario we are just making sure that the server we logged in should fall under this regex which is enough to verify that the server list is not blank or weird and ofcourse no need to check the server login again as we have already logged in the server with the exact URL.

[metacosm]

@amitkrout that test is not really useful though… what is it checking exactly? For example, if odo version was hardcoded to return a URL that matches the regexp, the test would pass… and it still wouldn't prove that odo version works as expected.

It'd be better, in my mind, to retrieve the server URL after logging into it (for example, parsing the output of oc project) and check that the URL returned by odo version matches that URL… because what matters is not the format of the value, but the value itself.

[amitkrout]

@amitkrout that test is not really useful though… what is it checking exactly? For example, if odo version was hardcoded to return a URL that matches the regexp, the test would pass… and it still wouldn't prove that odo version works as expected.

yup, for hardcoded values that falls under the re pattern won't report failure.

It'd be better, in my mind, to retrieve the server URL after logging into it (for example, parsing the output of oc project) and check that the URL returned by odo version matches that URL… because what matters is not the format of the value, but the value itself.

+1, It will be easy to parse and match machine readable output for example oc project -o json or may be oc version -o json. I will draft a separate pr for this against the issue #3342 , thanks.