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 stress-test workflow by adding grep functionality #35415

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

nemanjaglumac
Copy link
Member

@nemanjaglumac nemanjaglumac commented Nov 6, 2023

This change allows one to filter a specific test, or a set of tests to run within a single spec by using a Cypress grep functionality.

The documentation and examples are here:
https://github.com/cypress-io/cypress/tree/develop/npm/grep

Why?

Sometimes you just want to test a single test within a large spec. You'd do this locally in an "open mode" by appending .only() to the test but that doesn't work with Cypress grep.

If you tried this, you probably saw this error:

  1) An uncaught error was detected outside of a test

  0 passing (310ms)
  1 failing

  1) An uncaught error was detected outside of a test:
     TypeError: The following error originated from your test code, not from Cypress.

  > Cannot read properties of undefined (reading 'appendOnlyTest')

When Cypress detects uncaught errors originating from your test code it will automatically fail the current test.

Cypress could not associate this error to any specific test.

We dynamically generated a new test to display this failure.

Solution

By using a dynamic grep filtering that Cypress grep plugin allows, we have way more flexibility than if we had to hard code .only() to a test.

How to test this change?

The easiest and the most accurate way is to test it directly in CI. This is exactly what I did here.

  1. Give it a grep phrase as an input (I was using "native query" in `e2e/test/scenarios/visualizations/table.cy.spec.js) ✅
    https://github.com/metabase/metabase/actions/runs/6770593301/job/18399303406#step:9:168

It correctly skipped all other tests and ran only the one that contains that phrase in the name.

Running:  table.cy.spec.js                                                                (1 of 1)


  scenarios > visualizations > table
    - should allow changing column title when the field ref is the same except for the join-alias
    - should allow you to reorder and hide columns in the table header
    - should allow to display any column as link with extrapolated url and text
    - should show field metadata in a popover when hovering over a table column header
    - should show the field metadata popover for a foreign key field (metabase#[195](https://github.com/metabase/metabase/actions/runs/6770593301/job/18399303406#step:9:196)77)
    ✓ should show field metadata popovers for native query tables: burning 1 of 5 (16799ms)
    ✓ should show field metadata popovers for native query tables: burning 2 of 5 (15413ms)
    ✓ should show field metadata popovers for native query tables: burning 3 of 5 (11247ms)
    ✓ should show field metadata popovers for native query tables: burning 4 of 5 (11025ms)
    ✓ should show field metadata popovers for native query tables: burning 5 of 5 (10331ms)
    - should close the colum popover on subsequent click (metabase#16789)
    - popover should not be scrollable horizontally (metabase#31339)
    - default picker container should not be scrollable horizontally

  scenarios > visualizations > table > conditional formatting
    - should work with boolean columns
  1. Since grep input is not required, let's test this workflow by omitting grep altogether ✅
    https://github.com/metabase/metabase/actions/runs/6770736797

As expected, it ran all tests in that spec.

Running:  table.cy.spec.js                                                                (1 of 1)

  scenarios > visualizations > table
    ✓ should allow changing column title when the field ref is the same except for the join-alias: burning 1 of 2 (13161ms)
    ✓ should allow changing column title when the field ref is the same except for the join-alias: burning 2 of 2 (7761ms)
    ✓ should allow you to reorder and hide columns in the table header: burning 1 of 2 (7487ms)
    ✓ should allow you to reorder and hide columns in the table header: burning 2 of 2 (6860ms)
    ✓ should allow to display any column as link with extrapolated url and text: burning 1 of 2 (4972ms)
    ✓ should allow to display any column as link with extrapolated url and text: burning 2 of 2 (4946ms)
    ✓ should show field metadata in a popover when hovering over a table column header: burning 1 of 2 (8393ms)
    ✓ should show field metadata in a popover when hovering over a table column header: burning 2 of 2 (8631ms)
    ✓ should show the field metadata popover for a foreign key field (metabase#19577): burning 1 of 2 (2269ms)
    ✓ should show the field metadata popover for a foreign key field (metabase#19577): burning 2 of 2 (2126ms)
    ✓ should show field metadata popovers for native query tables: burning 1 of 2 (3618ms)
    ✓ should show field metadata popovers for native query tables: burning 2 of 2 (3629ms)
    - should close the colum popover on subsequent click (metabase#[167](https://github.com/metabase/metabase/actions/runs/6770736797/job/18399735654#step:9:168)89)
    ✓ popover should not be scrollable horizontally (metabase#31339): burning 1 of 2 (3712ms)
    ✓ popover should not be scrollable horizontally (metabase#31339): burning 2 of 2 (3633ms)
    ✓ default picker container should not be scrollable horizontally: burning 1 of 2 (4961ms)
    ✓ default picker container should not be scrollable horizontally: burning 2 of 2 (4994ms)
  scenarios > visualizations > table > conditional formatting
    ✓ should work with boolean columns: burning 1 of 2 (3574ms)
    ✓ should work with boolean columns: burning 2 of 2 (3381ms)

This change allows one to filter a specific test, or a set of tests to run
withing a single spec, by using a Cypress `grep` functionality.

The documentation and examples are here:
https://github.com/cypress-io/cypress/tree/develop/npm/grep
@nemanjaglumac nemanjaglumac requested a review from a team November 6, 2023 12:14
@nemanjaglumac nemanjaglumac self-assigned this Nov 6, 2023
Copy link
Contributor

@iethree iethree left a comment

Choose a reason for hiding this comment

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

💖 Can you also add this info to the ## How to stress-test a flake fix? section of e2e-tests.md ?

@nemanjaglumac
Copy link
Member Author

💖 Can you also add this info to the ## How to stress-test a flake fix? section of e2e-tests.md ?

Thank you for the reminder.
That documentation was already outdated so I used this chance to update it. Do you mind taking another look at his change only @iethree ?
aca9b32

@nemanjaglumac nemanjaglumac merged commit 370c36d into master Nov 6, 2023
@nemanjaglumac nemanjaglumac deleted the grep-stress-test-flake-fix branch November 6, 2023 15:05
@nemanjaglumac nemanjaglumac added the no-backport Do not backport this PR to any branch label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.CI & Tests no-backport Do not backport this PR to any branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants