Skip to content

Commit

Permalink
Examples tests revamp. (#7098)
Browse files Browse the repository at this point in the history
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
- [x] **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.
- [x] **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.
- [x] **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`.
- [x] **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.
- [x] **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
- [x] 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!
- [x] The tests now have significantly less complexity, only doing the
work that they really need to be doing.
- [x] 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.
- [x] They're faster. Locally testing is approximately 90% faster for me
while CI is about 50% faster. This likely a symptom of being simpler.
- [x] 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

---------

Co-authored-by: Chris Olszewski <[email protected]>
Co-authored-by: Mehul Kar <[email protected]>
  • Loading branch information
3 people authored Jan 31, 2024
1 parent 0ea86dc commit 720086b
Show file tree
Hide file tree
Showing 68 changed files with 710 additions and 604 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,10 @@ jobs:

- name: Check examples
shell: bash
run: turbo run test -F "@turborepo-examples-tests/*" --continue --color --remote-only --token=${{ secrets.TURBO_TOKEN }} --team=${{ vars.TURBO_TEAM }} --env-mode=strict --concurrency=1
# Concurrency being 1 is a big hammer here.
# It's a quick fix for non-deterministic behaviors we're seeing around package resolution.
# We could likely do some hacking to reparallelize this, but it's not worth it right now.
run: turbo run test --filter="@turborepo-examples-tests/*" --continue --token=${{ secrets.TURBO_TOKEN }} --team=${{ vars.TURBO_TEAM }} --env-mode=strict --concurrency=1

# Disable corepack again. actions/setup-node's "Post" step runs at the end of
# this job invokes other package managers, and corepack throws an error.
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ sweep.timestamp
*.t.orig
.cram_env

# Example tests artifacts
examples-tests-tmp/

# generated by tonic
file_descriptor_set.bin

Expand Down
1 change: 1 addition & 0 deletions examples/non-monorepo/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ next-env.d.ts

# turbo
.turbo
/tmp/
24 changes: 19 additions & 5 deletions examples/non-monorepo/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
{
"compilerOptions": {
"target": "es5",
"lib": ["dom", "dom.iterable", "esnext"],
"lib": [
"dom",
"dom.iterable",
"esnext"
],
"allowJs": true,
"skipLibCheck": true,
"strict": true,
Expand All @@ -19,9 +23,19 @@
}
],
"paths": {
"@/*": ["./*"]
}
"@/*": [
"./*"
]
},
"forceConsistentCasingInFileNames": true
},
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"],
"exclude": ["node_modules"]
"include": [
"next-env.d.ts",
"**/*.ts",
"**/*.tsx",
".next/types/**/*.ts"
],
"exclude": [
"node_modules"
]
}
2 changes: 2 additions & 0 deletions examples/with-gatsby/apps/web/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/** @type {import("eslint").Linter.Config} */
module.exports = {
extends: ["@repo/eslint-config/gatsby.js"],
ignorePatterns: ["gatsby-types.d.ts"],
};
8 changes: 1 addition & 7 deletions examples/with-gatsby/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion examples/with-react-native-web/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ yarn-error.log*
.env.production.local

# turbo
.turbo
.turbo
3 changes: 3 additions & 0 deletions examples/with-rollup/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ coverage
out/
build

# outputs
dist

# misc
.DS_Store
*.pem
Expand Down
1 change: 1 addition & 0 deletions examples/with-rollup/apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"react-dom": "^18.2.0"
},
"devDependencies": {
"@next/eslint-plugin-next": "^14.1.0",
"@repo/eslint-config": "workspace:*",
"@repo/typescript-config": "workspace:*",
"@types/node": "^20.10.6",
Expand Down
2 changes: 1 addition & 1 deletion examples/with-rollup/packages/config-eslint/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module.exports = {
],
overrides: [
{
files: ["*.js?(x)", "*.ts?(x)"],
files: ["*.js?(x)", "*.cjs?(x)", "*.mjs?(x)", "*.ts?(x)"],
},
],
};
4 changes: 2 additions & 2 deletions examples/with-rollup/packages/ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
"type": "module",
"exports": {
"./button": {
"types": "./dist/Button.d.ts",
"types": "./Button.tsx",
"default": "./dist/button.js"
},
"./header": {
"types": "./dist/Header.d.ts",
"types": "./Header.tsx",
"default": "./dist/header.js"
}
},
Expand Down
Loading

0 comments on commit 720086b

Please sign in to comment.