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

Examples tests revamp. #7098

Merged
merged 59 commits into from
Jan 31, 2024
Merged

Examples tests revamp. #7098

merged 59 commits into from
Jan 31, 2024

Conversation

anthonyshew
Copy link
Contributor

@anthonyshew anthonyshew commented Jan 25, 2024

The tests for examples have proven to be a point of friction for us. This PR aims to fix that.

Try it out locally with turbo test -F "@turborepo-examples-tests*" --continue --concurrency=5


There is a bit of technical debt associated with these tests, I'm told. Historically, these tests were used more like integration or end-to-end tests to ensure that turbo was behaving predictably on example monorepos.

Today, we're making the explicit commitment that these tests no longer need to serve that goal. Instead, they are for ensuring that the examples work correctly against the latest published version of turbo. We're making sure that users can trust that npx create-turbo@latest [-e <some-example>] will work out of the box.

Problems to solve

  • Complexity
    • It appears that our scripting once handled swapping out package managers for every example. We no longer feel the need to test our examples in this way. We trust that the rest of our CI has managed these complexities way before they make it to end users downloading an example.
    • There is also layers of scripting around platforms, git, and manipulating the package manager. I'm not totally sure what these were for - but they're not useful for our new goals.
  • Flakiness
    • I suspect that we're not actually surfacing broken examples in these tests. I've received reports from users that our examples didn't work out of the box - while these tests are green. 🧐 As we peel back the technical debt, I believe we will find out if this is indeed the case.
    • I've personally experienced these tests fail at unexpected times, pass at unexpected times, and pass and fail by re-running the same job multiple times with the same code.
    • Going back to our freshly stated goals, the tests catch problems way too late (when they do truly catch problems). The examples can potentially go untested against the latest published version of turbo indefinitely.
  • Testing the wrong thing
    • The tests are currently set up to do snapshot testing using Prysk. However, we no longer are interested in trying to find drift in logs in these tests so we don't need this log capturing layer.
    • This Prysk layer also adds a heavy, third layer of task runner management between terminal command and final package.json script execution. The other two are turbo.
  • Debuggability
    • Understanding and debugging the current test setup is extremely difficult. I have my suspicions that this also goes back to the Prysk layer as it makes logs quite hard to read. I've found it also swallows logs during the debugging process that would have been useful to see earlier.
    • I expect that this issue will fix itself as we address the complexity and flakiness concerns above.
  • Performance
    • I have a hunch that these tests can just...be faster. This will likely be a tailwind of addressing the aforementioned complexity.

Non-goals

  • There are a few examples that we don't test due to their own internal complexities (namely, Docker). We could choose to test these examples in the future but we're preferring to fix what we already have first.
  • We do not want to test the examples against turbo@HEAD.

Key changes

  • No more Prysk!: My first experiment was to get Prysk out of the way to see what was going on more clearly and, well, I kept not using it and kept getting closer to my goals.
  • No more snapshot tests: Because we're not concerned about drift in the examples, we can stop doing snapshot tests. This piggybacks on the "No more Prysk" idea.
  • Way less scripting: I love Bash just as much as the next developer. But, alas, the new codepaths use way less of it. Fewer steps, less work, trimming the fat of things we used to do that we no longer need to.

Results

  • These updates caught issues in the existing examples! I'm not certain how these issues were being covered up in the previous implementation but I've confirmed three important fixes that were being hidden. They were caught in CI and I fixed them. Early win!
  • The tests now have significantly less complexity, only doing the work that they really need to be doing.
  • From my experimenting, the tests are now deterministic. I can quickly reproduce issues on my local machine that I found in CI. I've also had success purposefully creating errors and resolving them.
  • They're faster. Locally testing is approximately 90% faster for me while CI is about 50% faster. This likely a symptom of being simpler.
  • They're more predictable, more debuggable, and I've included a README to ensure continued understanding of these tests in the future.

TODO?

  • We could choose to run these tests on a cron to ensure we're keeping up with published sooner. This PR still leaves us open to not testing the examples against latest. (This is slightly mitigated by Dependabot always running on Mondays. We would find out that latest broke an example then.)
  • We could also choose to run these tests as a part of the release process.

Closes TURBO-2177

Copy link

vercel bot commented Jan 25, 2024

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

Name Status Preview Comments Updated (UTC)
examples-kitchensink-blog 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 10:33pm
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 10:33pm
rust-docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 10:33pm
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 10:33pm
7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jan 30, 2024 10:33pm

Copy link
Contributor

github-actions bot commented Jan 25, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Jan 25, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details


# Head into a temporary directory
mkdir -p ../../examples-tests-tmp
cd ../../examples-tests-tmp
Copy link
Contributor Author

@anthonyshew anthonyshew Jan 29, 2024

Choose a reason for hiding this comment

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

We're going to work in a temporary directory for better test isolation. Ya know, like I actually know what I'm doing finally.

Copy link
Contributor

@mehulkar mehulkar left a comment

Choose a reason for hiding this comment

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

Mostly there, there's some script cleanup to do. I'd also install shellcheck as a VS Code extension if you don't have it already (it's a linter essentially), I've found that to be a pretty useful learning tool!

turborepo-tests/README.md Outdated Show resolved Hide resolved
turborepo-tests/helpers/setup_example_test.sh Show resolved Hide resolved
turborepo-tests/helpers/setup_example_test.sh Outdated Show resolved Hide resolved
turborepo-tests/helpers/setup_example_test.sh Outdated Show resolved Hide resolved
turborepo-tests/helpers/setup_example_test.sh Outdated Show resolved Hide resolved
turborepo-tests/helpers/setup_example_test.sh Outdated Show resolved Hide resolved
turbo_command="turbo build lint"

# Head into a temporary directory
mkdir -p ../../examples-tests-tmp
Copy link
Contributor

Choose a reason for hiding this comment

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

In case you didn't already know, there's a handy way of making a tmp directory (that you never need to touch again, the OS will just clean it up on restart):

xx=$(mktemp -d)
echo $xx # gives you the full path to the new dir)

https://linux.die.net/man/1/mktemp

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've handled everything but this. I can't seem to get the randomized directories to play nice with everything else. 🤔

This would be a small, small win for local debugging. I'm okay with punting it.

Copy link
Contributor

@mehulkar mehulkar left a comment

Choose a reason for hiding this comment

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

Ran it locally and the tests passed, nice job!

I don't love that it leaves a examples-tests-tmp/ laying around my repo which is largely a duplicate of examples/ , but I guess that's ok and/or we can take it another pass. (I'm concerned about things like the extra noise when using find locally, etc etc).

Approving since we've gone a few rounds on this and I'd rather discover any problems and iterate than try to hunt them all down now.

@anthonyshew anthonyshew merged commit 720086b into main Jan 31, 2024
53 of 58 checks passed
@anthonyshew anthonyshew deleted the examples-tests-revamp branch January 31, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ci area: examples Improvements or additions to examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants