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] run_robot_test.sh - OC login and return value #943

Merged
merged 3 commits into from
Sep 27, 2023

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Sep 21, 2023

This fixes OC login on my local Fedora 38 machine. Without this change,
the strings contain also ".

Also missing - in front of the switch isn't allowed.

Tested with Python based yq tool:

$ yq --version # https://github.com/kislyuk/yq
yq 3.2.3

$ yq --help
...
  -e               set the exit status code based on the output;
...
  -r               output raw strings, not JSON texts;
...

and also with GoLang based yq tool:

$ ./yq_linux_amd64 --version # https://github.com/mikefarah/yq
yq (https://github.com/mikefarah/yq/) version v4.35.2
$ ./yq_linux_amd64 --help
...
  -e, --exit-status                   set exit status if there are no matches or null or false is returned
...
  -r, --unwrapScalar                  unwrap scalar, print the value with no quotes, colors or comments. Defaults to true for yaml (default true)
...

@jstourac
Copy link
Member Author

Note - regarding the shellcheck, there is also this PR opened #936.

kornys
kornys previously approved these changes Sep 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2023

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
364 0 0 364 100

@jstourac
Copy link
Member Author

@tarukumar regarding our discussion whether to remove the e - if we remove it, then the script returns 0 even if the searched key isn't present in the provided variables file, see e.g.:

$ yq -r '.OCP_API_URLf' "ods_ci/test-variables.yml" ; echo $?
null
0
$ yq -er '.OCP_API_URLf' "ods_ci/test-variables.yml" ; echo $?
null
1

So, I guess we want to keep it there? Truth is that looking on the code, it won't stop execution even in case of failure. I can update that.

@jstourac
Copy link
Member Author

I added a commit with the check for the yq return value. Also rebased against master.

@jstourac
Copy link
Member Author

Note: yesterday I was able to test this on a Mac OS machine - these changes should work just fine.

Also note - on Mac OS the original code worked just fine, so probably just Linux users are affected with this. Looks like we have more Mac users than Linux users here (what a shame 😄 ).

@jstourac jstourac added verified This PR has been tested with Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Sep 25, 2023
@jstourac jstourac self-assigned this Sep 25, 2023
ods_ci/run_robot_test.sh Show resolved Hide resolved
@jstourac jstourac added needs testing Needs to be tested in Jenkins do not merge Do not merge this yet please and removed verified This PR has been tested with Jenkins labels Sep 26, 2023
@jstourac jstourac added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins do not merge Do not merge this yet please labels Sep 26, 2023
This fixes OC login on my local Fedora 38 machine. Without this change,
the strings contain also `"`.

Also missing `-` in front of the switch isn't allowed.

Tested with Python based `yq` tool:

```
$ yq --version # https://github.com/kislyuk/yq
yq 3.2.3

$ yq --help
...
  -e               set the exit status code based on the output;
...
  -r               output raw strings, not JSON texts;
...
```
and also with GoLang based `yq` tool:
```
$ ./yq_linux_amd64 --version # https://github.com/mikefarah/yq
yq (https://github.com/mikefarah/yq/) version v4.35.2
$ ./yq_linux_amd64 --help
...
  -e, --exit-status                   set exit status if there are no matches or null or false is returned
...
  -r, --unwrapScalar                  unwrap scalar, print the value with no quotes, colors or comments. Defaults to true for yaml (default true)
...
```
Return value in case of service account login could be wrong
@sonarcloud
Copy link

sonarcloud bot commented Sep 26, 2023

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
No Duplication information No Duplication information

@jstourac
Copy link
Member Author

Rebased.

Also created #953.

@bdattoma bdattoma merged commit d498907 into red-hat-data-services:master Sep 27, 2023
8 checks passed
@jstourac jstourac deleted the runScriptFixes branch September 27, 2023 09:21
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Nov 28, 2023
…vices#943)

* [Fix] run_robot_test.sh - OC login

This fixes OC login on my local Fedora 38 machine. Without this change,
the strings contain also `"`.

Also missing `-` in front of the switch isn't allowed.

Tested with Python based `yq` tool:

```
$ yq --version # https://github.com/kislyuk/yq
yq 3.2.3

$ yq --help
...
  -e               set the exit status code based on the output;
...
  -r               output raw strings, not JSON texts;
...
```
and also with GoLang based `yq` tool:
```
$ ./yq_linux_amd64 --version # https://github.com/mikefarah/yq
yq (https://github.com/mikefarah/yq/) version v4.35.2
$ ./yq_linux_amd64 --help
...
  -e, --exit-status                   set exit status if there are no matches or null or false is returned
...
  -r, --unwrapScalar                  unwrap scalar, print the value with no quotes, colors or comments. Defaults to true for yaml (default true)
...
```

* [Fix] run_robot_test.sh - fix return value

Return value in case of service account login could be wrong

* [Fix] run_robot_test.sh - let's check return code of yq before oc login
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Jan 2, 2024
…vices#943)

* [Fix] run_robot_test.sh - OC login

This fixes OC login on my local Fedora 38 machine. Without this change,
the strings contain also `"`.

Also missing `-` in front of the switch isn't allowed.

Tested with Python based `yq` tool:

```
$ yq --version # https://github.com/kislyuk/yq
yq 3.2.3

$ yq --help
...
  -e               set the exit status code based on the output;
...
  -r               output raw strings, not JSON texts;
...
```
and also with GoLang based `yq` tool:
```
$ ./yq_linux_amd64 --version # https://github.com/mikefarah/yq
yq (https://github.com/mikefarah/yq/) version v4.35.2
$ ./yq_linux_amd64 --help
...
  -e, --exit-status                   set exit status if there are no matches or null or false is returned
...
  -r, --unwrapScalar                  unwrap scalar, print the value with no quotes, colors or comments. Defaults to true for yaml (default true)
...
```

* [Fix] run_robot_test.sh - fix return value

Return value in case of service account login could be wrong

* [Fix] run_robot_test.sh - let's check return code of yq before oc login
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants