Skip to content

ROX-12893: Wait for more requests in configmanagement integration tests#3268

Merged
pedrottimark merged 14 commits intomasterfrom
ROX-12893-cypress-configmanagement-wait
Oct 3, 2022
Merged

ROX-12893: Wait for more requests in configmanagement integration tests#3268
pedrottimark merged 14 commits intomasterfrom
ROX-12893-cypress-configmanagement-wait

Conversation

@pedrottimark
Copy link
Contributor

@pedrottimark pedrottimark commented Sep 30, 2022

Description

Test failure

  1. 1567400461261082624 from master build for Bump k8s.io/kubelet from 0.22.13 to 0.22.15 #3167 on 2022-09-26

Config Management Entities (Nodes) should render the nodes list and open the side panel with the clicked cluster value

Timed out retrying after 5000ms: cy.wait() timed out waiting 5000ms for the 1st request to the route: entities. No request ever occurred.

nodes-test-failure

A test-specific request times out while there is a problem with generic requests.

Analysis

This is only the second investigation to examine the build-log.text file. The first was in #2799

Compare 3 occurrences of refreshing auth token:

  • Occurrence 2 has long 31 second duration for the test which precedes the failure.
  • Occurrence 3 has similar 8 second durations for tests when refresh occurs.

Circumstantial evidence suggests that Refreshing the GKE auth token affects tests that call cy.visit method (which does not wait for any requests, which have problems in the picture above) instead of visit helper functions (which do wait for generic and container-specific requests).

  1. Success: certexpiration.test.js which calls visitMainDashboard helper function

    Cert Expiration Banner
        Central
    INFO: Mon Sep 26 17:25:48 UTC 2022: Refreshing the GKE auth token
        ✓ should not display banner if central cert is expiring more than 14 days later
    Fetching cluster endpoint and auth data.
    kubeconfig entry generated for rox-ci-ui-e2e-test-1574440640899452929.
        ✓ should display banner without download button if user does not have the required permission
    
  2. Failure: configmanagement/nodes.test.js which calls cy.visit method

    Config Management Entities (Nodes)
        ✓ should render the nodes list and open the side panel when a row is clicked (31340ms)
        1) should render the nodes list and open the side panel with the clicked cluster value
        ✓ should click on the cluster entity widget in the side panel and match the header  (14157ms)
    
    INFO: Mon Sep 26 17:40:52 UTC 2022: Refreshing the GKE auth token
    Fetching cluster endpoint and auth data.
    kubeconfig entry generated for rox-ci-ui-e2e-test-1574440640899452929.
    
  3. Success: policies/policyWizardStep3.test.js which calls visitPolicies helper function

    Policy wizard, Step 3 Policy Criteria
    
            ✓ should populate table modal select and respect changed values on save (8578ms)
    INFO: Mon Sep 26 17:56:00 UTC 2022: Refreshing the GKE auth token
            ✓ should populate table modal select and not change values on cancel (8763ms)
    Fetching cluster endpoint and auth data.
    kubeconfig entry generated for rox-ci-ui-e2e-test-1574440640899452929.
            ✓ should go to link when table row is clicked (8002ms)
    

In this test run, there are 3 occurrences at 15 minute intervals. It seems like there could be twice as many, because test runs last for 90 to 120 minutes.

Solution

  1. Replace cy.visit with visit functions. After these changes: 57 results in 18 files, some of which are skipped. That is, configmanagement folder had the largest number of occurrences, therefore would have been next in line, except that it is an abandoned container. However, CI did not get the memo to give it a break.
  2. Replace intercept-interaction-wait sandwiches with interact helper functions.
  3. Change or add arguments of existing helper functions so they can call interact functions.

Change summary

  1. Edit cypress/constants/ConfigManagementPage.js

    • Delete page addresses so helpers file can encapsulate them.
  2. Edit cypress/constants/apiEndpoints.js

    • Delete endpoint addresses so helpers file can encapsulate them.
  3. Edit cypress/helpers/configWorkflowUtils.js

    • Use plural page address segments as keys:
      • in local objects for endpoint or page address segments
      • in local objects for page text
    • Export visit and interact helper functions which have defense in depth against timing problems:
      • wait for primary request
      • assert either page address or DOM element, or both
    • Add arguments for entities to existing helper functions
  4. Edit test files

    • Add assignment statement for entitiesKey as an example, but comment it out to minimize changed lines.
    • Replace triggerScan for Compliance scan with interactAndWaitForConfigurationManagementScan function.
    • Delete clickOnRowEntity because redundant with entityListCountMatchesTableLinkCount function calls.

Lessons learned

  1. Inconsistencies increase the cost to wait on requests and assert DOM elements in helper functions (see the next suggestion). I had to step manually through every combination of primary and secondary entities with elements and network panels to find exceptions to patterns for GraphQL opnames and page text. This was also a big barrier when I replaced cy.server and cy.request with cy.intercept a year ago.
  2. Unique idioms are another barrier to understanding. Or evidence of misunderstanding. This is the only container whose tests use context(description, callback) blocks. However, it is a synonym for describe which is a parent of it not a child of it (pardon pun). Therefore, a test failure does not include the contextual string. The only reason why I did not split dual-context tests into pairs of tests is to minimize changed.
  3. Although the original configmanagement tests provided an early example of helper functions, which the later vulnmanagement tests did not follow, it was hard to see the forest for the trees. Here are the top 2 critiques:
    • There is little parallelism in coverage between test files. Many existing helper functions are called in fewer than half of test files, yet it seems unlikely that they were not relevant for most of the other half.
    • In addition, the order of tests does not follow workflow scenarios. Although most tests start with an entities list, the few tests of links from the table are at the end of test files. In comparison, a strength of vulnmanagement test files is that a test of table column headings is near the beginning.

Suggestion

For future work on workflow containers, provide the following in the container (not globally) with key of path segment for plural entities like clusters.

  1. Internal strings:

    • opname for plural entities request is same as path segment like clusters
    • opname for singular entity request like cluster
      If possible, supersede singular path segments (to distinguish entity page from classic side panel) with the usual plurals whatevers/id or primaries/id1/secondaries/id2, if 2 levels continue in the future.
    • internal constant like CLUSTER if required for query payload variables
      Do not use in query strings of page addresses if breadcrumb context continues into the future (separate from side panel as presentation page architecture). That is, use same clusters in query string as for path segment.
  2. External strings do not transform internal strings with capitalize, pluralize and so on for use as page text!

    • Sentence case for plural entities like Clusters for page heading, widget heading, table column heading
    • Sentence case for singular entity, if needed, like Cluster for widget heading
    • lowercase for plural entities like clusters for link text
    • lowercase for singular entity like cluster for link text

If UI has a clear and simple source of truth for each type of entities, then integration tests are easier to understand and helper functions are easier to write.

Checklist

  • Investigated and inspected CI test results
  • Edited integration tests

Testing Performed

  • Ran changed tests one at a time (some fail in local deployment)
  • Used elements and network panels with demo and staging for some scenarios

@pedrottimark pedrottimark changed the title Rox 12893 cypress configmanagement wait ROX-12893: Wait for more requests in configmanagement integration tests Sep 30, 2022
@ghost
Copy link

ghost commented Sep 30, 2022

Images are ready for the commit at 859fe34.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.72.x-202-g859fe34d73.

@pedrottimark
Copy link
Contributor Author

/test ui-unit-tests

1 similar comment
@pedrottimark
Copy link
Contributor Author

/test ui-unit-tests

@pedrottimark pedrottimark requested a review from vjwilson October 3, 2022 20:18
Copy link
Contributor

@vjwilson vjwilson left a comment

Choose a reason for hiding this comment

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

Looks good as-is, but I would go ahead and use the constants you have commented.

import withAuth from '../../helpers/basicAuth';
import { triggerScan } from '../../helpers/compliance';

// const entitiesKey = 'controls'; // omit to minimize changed lines
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure it's worth delaying this change by commenting this pattern in each of the files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will open a follow up use constants and to split it with pair of context into pair of it tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants