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

Build: Add e2e-tests-dev task #21546

Merged
merged 16 commits into from
Mar 24, 2023
Merged

Build: Add e2e-tests-dev task #21546

merged 16 commits into from
Mar 24, 2023

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Mar 10, 2023

What I did

We have been running e2e tests against built Storybooks, but not against Storybooks in dev mode.
This PR solves that!

How to test

Run a e2e-tests-dev for template, e.g. yarn task --task e2e-tests-dev --start-from auto --template react-vite/default-ts

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@yannbf yannbf added build Internal-facing build tooling & test updates ci:daily Run the CI jobs that normally run in the daily job. ci:merged Run the CI jobs that normally run when merged. and removed ci:daily Run the CI jobs that normally run in the daily job. labels Mar 10, 2023
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Can we factor things so we share the code between the two files?

We could have them read the same lib.

We could also consider a more complicated system where we can run any of the final tasks against either dev or build (arguably we could also run the test-runner task against dev also?) and have a flag that chooses between them, or a e2e-tests-dev task that depends on dev and e2e-tests somehow? Probably not the right time to think about that!

scripts/task.ts Outdated Show resolved Hide resolved
scripts/tasks/dev.ts Outdated Show resolved Hide resolved
@tmeasday
Copy link
Member

Also, let's make a ticket where we delete smoke testing from the repo if we all agree we don't need it any more.

@yannbf
Copy link
Member Author

yannbf commented Mar 11, 2023

Can we factor things so we share the code between the two files?

We could have them read the same lib.

We could also consider a more complicated system where we can run any of the final tasks against either dev or build (arguably we could also run the test-runner task against dev also?) and have a flag that chooses between them, or a e2e-tests-dev task that depends on dev and e2e-tests somehow? Probably not the right time to think about that!

Would you like to take a stab at it @tmeasday? Feel free to change this PR or make a telescoping one!

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

@tmeasday @yannbf Can we limit our dev coverage to 1 webpack-based project and 1 vite-based project and perhaps 1-2 others at most? If not, we're nearly doubling our CI costs, right? If we adopt nx affected and are able to dramatically reduce costs that way, we can look at testing dev more aggressively. WDYT?

@yannbf yannbf force-pushed the chore/add-e2e-tests-for-dev-mode branch from e3b9acf to 01e21c9 Compare March 11, 2023 13:56
@yannbf
Copy link
Member Author

yannbf commented Mar 11, 2023

@tmeasday @shilman I worked on it based on your feedback. Please check!

@yannbf yannbf removed the ci:merged Run the CI jobs that normally run when merged. label Mar 11, 2023
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM. We should talk about a better way to disable a given task at a given cadence but this way is good for now!

.circleci/config.yml Outdated Show resolved Hide resolved
@yannbf yannbf force-pushed the chore/add-e2e-tests-for-dev-mode branch 2 times, most recently from 01e21c9 to 337d72d Compare March 20, 2023 14:55
@yannbf yannbf force-pushed the chore/add-e2e-tests-for-dev-mode branch from 337d72d to d2b8916 Compare March 20, 2023 15:41
@yannbf
Copy link
Member Author

yannbf commented Mar 20, 2023

I'd love some help to understand why these tests fail in CI:

image

@tmeasday
Copy link
Member

@yannbf if you are stuck I can have a go at figuring it out tomorrow perhaps?

@yannbf
Copy link
Member Author

yannbf commented Mar 21, 2023

@yannbf if you are stuck I can have a go at figuring it out tomorrow perhaps?

Thank you!

@tmeasday tmeasday force-pushed the chore/add-e2e-tests-for-dev-mode branch from 1500e63 to 231aee4 Compare March 23, 2023 06:08
@yannbf yannbf added the ci:daily Run the CI jobs that normally run in the daily job. label Mar 23, 2023
@yannbf yannbf removed the ci:daily Run the CI jobs that normally run in the daily job. label Mar 24, 2023
@yannbf yannbf merged commit 2b834fc into next Mar 24, 2023
@yannbf yannbf deleted the chore/add-e2e-tests-for-dev-mode branch March 24, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Internal-facing build tooling & test updates
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants