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

Improve E2E tests #12366

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Conversation

richard-cox
Copy link
Member

@richard-cox richard-cox commented Oct 25, 2024

Summary

Contributes to #12356

Needs to be squash merged

Occurred changes and/or fixed issues

  • LOTS of improvements, including reliability
  • Flakyness hasn't completely been fixed (see below), but this is a large step in the right direction (only 1 bump required..)

Generally

  • fixed / improved some rancher-api-commands
    • fix for likely cause of some resources not being deleted)
    • two fns that change userpreferences now wait for the expected value to be fetched before moving on
      • this is the big one. somehow the PUT was being ignored or overwritten
      • this meant often
        • the state within the test was not what it should be, resulting in failures
        • the state was not left as it should be, which had knock on affects to other tests
      • also it was a silent failure. tests would just move on
      • fixes
        • check if the state is set correctly
        • retry process if state doesn't get set properly
        • fail is state still isn't set properly!
  • improving resource names in a lot of places
    • include run id when resource created
  • allow more time for fleet to come up before trying to nav to it
  • improved reliability of list tests
    • running with and without vai on, mostly around counts
  • remove junk revision from mock resources. invalid revisions are rejected via socket watch... resulting in UI SPAMMING requests to mock resources trying to find a valid revision. this causes CPU churn and slower tests
  • burger menu po updated to make clear, there's pinned and non-pinned cluster lists (works around badly saved pinned cluster userpreferences)
  • comment out it.skip tests. these cause havok with test reports

Specifically

  • repositories.spec.ts - ensure start up churn settles down before counting repos
  • horizontal-pod-autoscalers.spec.ts - user setting isn't sticking (setup but lost/not supplied in refresh). work around
  • gitrepo.spec.ts - select language via name rather than index (not sure why we wouldn't have all languages as before..)
  • settings.spec.ts - wait for account and api keys in avatar menu to be visible before starting test (could be vue reactivity issue)
  • kontainer-drivers.spec.ts - scroll things into view
  • users-and-auth/users.spec.ts - scroll things into view
  • api-services.spec - table can update with more rows, so use fixed amount of rows to select
  • roles.spec - filter by a specific namespace in case socket updates result add new rows
  • and much much more...

Not fixed

  • still some sketchyness around list based tests (if count checks are incorrect we should update others to follow pattern in crd spec)
  • still some sketchyness around userpreferences. it can sometimes error with something like namespaces already exist(?!)

Technical notes summary

For adding to guide - see #12483

  • Test should not take starting condition for granted. State should be setup if required and confirmed before starting
    • tests involve checking for pods in cattle-system (has a lot of flex)
  • Any state set for the test should be reverted afterwards (resources created, preferences applied, etc)
  • The name for created Resources should come from createE2EResourceName (lots of examples available)
  • Creating resources via blueprint / stock yaml should be seen as last resort
    • These are very brittle and verbose
    • Resources should be dynamically created via api before the test (and cleaned up afterwards)
  • When blueprint / stock yaml HAS to be used ensure resourceRevision and revision are set to CYPRESS_SAFE_RESOURCE_REVISION
    • When these are set low there's a high chance rancher will reject the watch from it, resulting in UI re-fetching the same stock resource with bad revision --> watch rejected --> fetch bad revision --> spam which can throttle CPU. problem is much worst when running tests against existing instance.
  • Avoid using .skip to temporarily disable tests and instead comment them out.
    • skip causes havoc with test results

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@richard-cox richard-cox changed the title Improve pod.spec test Improve E2E tests Oct 30, 2024
@nwmac nwmac modified the milestones: v2.10.0, v2.11.0 Oct 31, 2024
@richard-cox richard-cox marked this pull request as ready for review November 1, 2024 13:52
@richard-cox richard-cox mentioned this pull request Nov 4, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants