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.

Sign up for GitHub

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

Jump to bottom

ci(examples): make every examples test a workspace #5011

Merged
merged 9 commits into from
Jun 9, 2023
Merged

ci(examples): make every examples test a workspace #5011

merged 9 commits into from
Jun 9, 2023

Conversation

mehulkar
Copy link
Contributor

@mehulkar mehulkar commented May 17, 2023 •
edited
Loading

Summary

Parallelize examples tests with turbo instead of Github Actions.

(This is an alternative approach to parallelizing with pytest+prysk in #4190)

Why

  • Reduce the number of Github Workflow runs triggered when an example
    changes. This should be faster and easier to read in a PR (2 checks instead of 16).
  • More turbo dogfooding

Caveats

All examples tests are still run when any of the examples change, because each
example itself is not a workspace, so we cannot wire up individual dependencies.
It might make sense to put this test inside each examples/, but since people
download these examples (via npx create-turbo), we'd need to be careful about
not including the tests into the example.

Reviewer notes

  • turbo.json configures inputs for caching, but we still using the Github Actions
    config to to limit these runs to changes in examples/ and examples-tests/**.
    We could opt out of this and rely on turbo's caching, but I'm deferring that for now.

  • Some tests don't really make sense to me (e.g. npm with the with-yarn example)
    but I'm not planning to change/delete those in this PR, unless I've translated the
    matrix wrong.

  • The pnpm-gatsby test was not part of the matrix before. Moving to this setup revealed
    that it does not pass in CI (because of mismatched node version). I'm keeping the test,
    but filtering it out in CI, and will take a separate pass at making it pass. It already has another
    issue where it doesn't get "FULL TURBO".

Recent work in this area:

@vercel
Copy link

vercel bot commented May 17, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

11 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-cra-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview Jun 8, 2023 5:38pm

@mehulkar mehulkar mentioned this pull request May 17, 2023
Closed
@github-actions

This comment was marked as off-topic.

Sign in to view
@github-actions

This comment was marked as off-topic.

Sign in to view
@tknickman
Copy link
Member

There are some funky tests that don't really make sense to me
(e.g. npm with the with-yarn). I'm not planning to change/delete those
in this PR, unless I've translated the matrix wrong.

We don't even really need the with-yarn or with-npm examples anymore since create-turbo always uses basic and just codemods between the package managers. The only reason I left them was for those who may prefer browsing an example before getting started. But I could be convinced to yank them out.

@github-actions

This comment was marked as off-topic.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented May 18, 2023 •
edited
Loading

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)
  • Turbopack Rust benchmark tests (mac/win, non-blocking)

See workflow summary for details

@mehulkar mehulkar marked this pull request as ready for review May 18, 2023 20:39
@mehulkar mehulkar requested review from a team as code owners May 18, 2023 20:39
@mehulkar mehulkar force-pushed the mehulkar/turbo-1109-parallelize-examples-in-a-single branch from b29ebe7 to e5ccb74 Compare May 18, 2023 20:47
@github-actions

This comment was marked as off-topic.

Sign in to view
@mehulkar mehulkar force-pushed the mehulkar/turbo-1109-parallelize-examples-in-a-single branch from e5ccb74 to da4c3ec Compare May 18, 2023 23:53
@github-actions

This comment was marked as off-topic.

Sign in to view
@mehulkar
Copy link
Contributor Author

@tknickman yeah I'm ok to add/remove as well. Let's figure that out outside this PR

@github-actions

This comment was marked as off-topic.

Sign in to view
@github-actions

This comment was marked as off-topic.

Sign in to view
@mehulkar
ci: make every examples test a workspace
806ec8e 
This setup will allow turbo to parallelize and cache execution of these
tests. It also reduces the number of Github Workflow runs that are
triggered when an example changes, greatly reducing the amount of
computing power required to run the tests.

This commit does not change the fact that all examples tests are run
when any of the examples change. We may be able to wire up dependencies
more carefully in the future, but this is not feasible at the moment
because each example itself is not a workspace.
@mehulkar mehulkar force-pushed the mehulkar/turbo-1109-parallelize-examples-in-a-single branch from 8aed70b to 806ec8e Compare May 22, 2023 20:35
@github-actions

This comment was marked as off-topic.

Sign in to view
@github-actions

This comment was marked as off-topic.

Sign in to view
This was referenced Jun 8, 2023
Merged
Merged
@mehulkar mehulkar requested review from gsoltis, chris-olszewski, NicholasLYang and arlyon and removed request for nathanhammond June 8, 2023 17:15
@mehulkar mehulkar changed the title ci: make every examples test a workspace ci(examples): make every examples test a workspace Jun 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

Linux Benchmark for 4daeaaa

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 10.22ms ± 0.09ms 10.17ms ± 0.07ms -0.51%
bench_hmr_to_eval/Turbopack CSR/1000 modules 9236.73µs ± 95.98µs 9208.15µs ± 84.85µs -0.31%
bench_startup/Turbopack CSR/1000 modules 1028.52ms ± 3.34ms 1025.24ms ± 6.37ms -0.32%

@mehulkar mehulkar merged commit 177fc21 into main Jun 9, 2023
@mehulkar mehulkar deleted the mehulkar/turbo-1109-parallelize-examples-in-a-single branch June 9, 2023 04:51
@mehulkar mehulkar mentioned this pull request Jun 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsoltis gsoltis gsoltis approved these changes

@tknickman tknickman Awaiting requested review from tknickman tknickman is a code owner automatically assigned from vercel/turbo-oss

@chris-olszewski chris-olszewski Awaiting requested review from chris-olszewski

@NicholasLYang NicholasLYang Awaiting requested review from NicholasLYang

@arlyon arlyon Awaiting requested review from arlyon

Assignees
No one assigned
Labels
area: ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mehulkar @tknickman @gsoltis