docs: a contributor following CONTRIBUTING ran 64% of the tests - #286
Merged
Conversation
Cloned the repository fresh and did exactly what the guide says. It reports success: Tasks: 28 successful 573 passed, 608 skipped Every test touching Postgres is guarded by skipIf(!hasDb), so with no DATABASE_URL more than half the suite quietly does not run — scoring, tenant isolation, erasure, the queue. A contributor could change any of it, see every task green, and have exercised none of what they changed. For a project whose whole subject is checks that look green without proving anything, that is the wrong first experience to hand someone. The guide now gives the command that reaches the database compose already started, with REQUIRE_DB=1 so an unreachable database fails loudly rather than skipping. With it: 895 passed, 0 skipped. It also documents `turbo run test --force`, which runs packages concurrently and uncached — how the Prisma upgrade was caught writing to the wrong schema — and says format:check is the step most often forgotten. The changeset list was wrong in both directions: it named `ai`, which is private, and omitted the Vitest and Jest reporters, which publish. Someone following it would either write a changeset nobody needs or miss one that gates a release. A test now checks the list against the workspace. That test first flagged `db`, which the section names as an example of something needing no changeset — the check was reading the prose rather than the list, so it now reads only the list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #78, whose acceptance criterion is "a first-time contributor can pick a task and open a PR without asking how to set up". I tested that the only way it can be tested: cloned the repository fresh and did exactly what the guide says.
It reports success while skipping half the suite
Every test touching Postgres is guarded by
describe.skipIf(!hasDb). With noDATABASE_URL, more than half the suite quietly does not run — scoring, tenant isolation, erasure, the queue. A contributor could change any of it, watch every task go green, and have exercised none of what they changed.For a project whose entire subject is checks that look green without proving anything, that is a poor first experience to hand someone.
With the database the guide already tells them to start, plus
REQUIRE_DB=1so an unreachable database fails loudly instead of skipping:REQUIRE_DB=1earned its place during this: I ran it against a stopped Postgres by accident and got a loudP1001instead of a green run. That is the whole point of it, and it was documented nowhere.The changeset list was wrong in both directions
It named
ai— private, never published — and omittedvitest-reporterandjest-reporter, which do. Someone following it would either write a changeset nobody needs or miss one that gates a release.A test now checks the list against the workspace. It first flagged
db, which the section names as an example of something needing no changeset — the check was reading the surrounding prose rather than the list, so it now reads only the list. Proved by removingvitest-reporterand watching it fail.Also documented
pnpm exec turbo run test --force, which runs packages concurrently and uncached — the check that caught the Prisma upgrade writing to the wrong schema, where serial cached runs were perfectly green. And thatformat:checkis the step most often forgotten.62/62 turbo tasks.