Skip to content
This repository has been archived by the owner on Jun 28, 2023. It is now read-only.

Improvements in smoke tests #2517

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

aman556
Copy link
Contributor

@aman556 aman556 commented Nov 11, 2021

What this PR does / why we need it

This PR is a follow-up PR to #2246.The changes in this PR are:

  • Smoke tests under test/smoke are improved as per the review comments in the last PR.
  • Gatekeeper test which is in test/gatekeeper is hooked in the test/smoke

Details for the Release Notes (PLEASE PROVIDE)

These are the improved smoke tests for the sanity test of the packages.

Which issue(s) this PR fixes

Fixes: # NA

Describe testing done for PR

make smoke-test is triggered to run all the smoke tests.

Special notes for your reviewer

Please refer to #2246 for complete picture.

@aman556 aman556 requested a review from a team as a code owner November 11, 2021 15:16
@github-actions github-actions bot added owner/docs Work executed by VMware documentation team owner/release-eng Work executed by VMware release engineering team labels Nov 11, 2021
Copy link
Contributor

@karuppiah7890 karuppiah7890 left a comment

Choose a reason for hiding this comment

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

LGTM ! I just have some thoughts on a few things to simplify the code and some nitty gritty things. I'll also execute and try this out next async to the review

test/smoke/Makefile Outdated Show resolved Hide resolved
test/smoke/packages/contour/1.18.2/contour-test.sh Outdated Show resolved Hide resolved
@karuppiah7890
Copy link
Contributor

In short, these are some things we could do as part of this PR or another PR

  • using kubectl wait instead of for loop with timeout calculation
  • adding set -o ...
    • taking care of errors and exit codes using ||
  • using kubectl get -o jsonpath or kubectl get -o template for getting status, phase
  • reset to white color after red error color
  • check port 80 root access - use sudo or different port if it's an issue
  • remove pid (ideally this PR as this can cause unnecessary problems)
  • remove trap (ideally this PR as this can cause unnecessary problems)

- Kubectl wait command is used
- adding set -o
- using kubectl get -o jsonpath or kubectl get -o template for getting status, phase
- Change the port from 80 to 8080

Signed-off-by: Aman Sharma <[email protected]>
@aman556 aman556 force-pushed the smoke-tests-improvement branch from d5e552d to 2478ea8 Compare November 12, 2021 11:01
Copy link
Contributor

@karuppiah7890 karuppiah7890 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me! Great work 😄 We can merge this, let me know what you think about the kubectl wait error case alone

@aman556
Copy link
Contributor Author

aman556 commented Nov 12, 2021

thanks @karuppiah7890 agree with the kubectl wait error issue as i have some other scripts to add so thinking to do this change in next PR.

@karuppiah7890
Copy link
Contributor

Makes sense! 👍

@karuppiah7890
Copy link
Contributor

Btw, the port thing, I didn't realize that it was just a service exposing it. I was thinking it was a port-forward, sorry! Anyways, 8080 is fine too 😅

@karuppiah7890 karuppiah7890 merged commit 326650e into vmware-tanzu:main Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-not-required owner/docs Work executed by VMware documentation team owner/release-eng Work executed by VMware release engineering team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants