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

feat(ci): Balance frontend tests #30949

Merged
merged 21 commits into from Jan 10, 2022
Merged

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Jan 6, 2022

This adds a jest reporter to record the test durations of our frontend tests. It also changes our runner to look for a results JSON and attempt to balance tests between our CI runners.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2022

size-limit report

Path Base Size (d66742b) Current Size Change
src/sentry/static/sentry/dist/entrypoints/app.js 53.5 KB 53.51 KB +0.02% 🔺
src/sentry/static/sentry/dist/entrypoints/sentry.css 68.38 KB 68.38 KB 0%

jest.config.ts Outdated Show resolved Hide resolved
}
}

module.exports = TestBalancer;
Copy link
Member

Choose a reason for hiding this comment

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

Can you write this in typescript as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not possible unfortunately: jestjs/jest#8810

@billyvg
Copy link
Member Author

billyvg commented Jan 6, 2022

Here's an example of frontend tests have wildly differing test sharding:

frontend (0) taking 23 minutes
frontend (1) taking 17 minutes

@billyvg
Copy link
Member Author

billyvg commented Jan 6, 2022

I think this is useful enough to merge as-is - ideally we would add some automation to update the balance JSON file. But I think even without automating it, this should suffice for awhile (until we add new tests that are quite slow).

Some automation ideas:

  • Scheduled workflow that runs the frontend tests and generates the results json
  • Generate the json while running tests, upload as GHA artifacts, and for each test run, pull from GHA artifacts and merge/generate the json (since test runs are sharded, they will have separate result json)

@billyvg billyvg marked this pull request as ready for review January 6, 2022 16:52
@billyvg billyvg requested a review from a team January 6, 2022 16:54
Copy link
Member

@evanpurkhiser evanpurkhiser left a comment

Choose a reason for hiding this comment

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

looks great, I like this.

@billyvg
Copy link
Member Author

billyvg commented Jan 6, 2022

Not sure what's up with the jest snapshots, but the tests are definitely getting run:
image

Copy link
Member

@narsaynorath narsaynorath left a comment

Choose a reason for hiding this comment

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

🤞


mountGlobalModal();
screen.getByTestId('widget-add').click();
Copy link
Member

Choose a reason for hiding this comment

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

should this be userEvent.click(screen.getByTestId('widget-add')) ?


mountGlobalModal();
screen.getByTestId('widget-add').click();

// Add a custom widget to the dashboard
(await screen.findByText('Custom Widget')).click();
Copy link
Member

Choose a reason for hiding this comment

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

here too?

@billyvg billyvg merged commit 9e9e5a4 into master Jan 10, 2022
@billyvg billyvg deleted the feat/ci/jest-reporter-for-test-times branch January 10, 2022 20:09
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants