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

Some updates to handle different cases for e2e run #226

Merged
merged 1 commit into from
May 19, 2023
Merged

Conversation

bobzilladev
Copy link
Contributor

@bobzilladev bobzilladev commented May 15, 2023

What

Changes that made the e2e tests work for me, these may not be universally applicable, so happy to cherry pick anything that is actually useful.

How

  • Typo in developer guide
  • Kustomization changes in e2e-fixtures/hello-world-ingress to remove bezek-hello-world-ingress-2 and set via config
  • e2e-fixtures/ingress-class 404 is HTTP/2 for me
  • The secrets keys don't seem to have NGROK prefix, unlike the environment variables they set
  • jq and yq weren't installed on this machine, added to nix config, and added some bash to check for them up front. looks like josh is adding nix support though, so this may become irrelevant.
  • Changed the test for GOOGLE_CLIENT_ID to not error out if not defined, due to the set -eu -o pipefail
  • A message before the long sleep to make the lack of output more expected

Breaking Changes

N/A

@github-actions github-actions bot added area/helm-chart Issues dealing with the helm chart documentation Improvements or additions to documentation testing labels May 15, 2023
@alex-bezek
Copy link
Collaborator

So i was the only one using these for the most part and near the end of the sprints things changed heavily and these got out of date. They still have the old annoation system we had before we used crds https://github.com/ngrok/kubernetes-ingress-controller/blob/main/e2e-fixtures/route-modules/route-modules.yaml#L6-L11

We may want to chat with you and the team about if you find these useful or if there are some changes we can make to make things a bit better. I think these tests end up having various state problems and actually were hitting api rate limits quite frequently

Copy link
Contributor

@nikolay-ngrok nikolay-ngrok left a comment

Choose a reason for hiding this comment

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

lgtm

@bobzilladev bobzilladev marked this pull request as ready for review May 19, 2023 21:02
@bobzilladev bobzilladev requested a review from a team as a code owner May 19, 2023 21:02
@bobzilladev bobzilladev merged commit 6ba4034 into main May 19, 2023
@bobzilladev bobzilladev deleted the bob/e2e branch May 19, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm-chart Issues dealing with the helm chart documentation Improvements or additions to documentation testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants