Repository navigation
Conversation
Add a shared @/core/testing/contract harness (renderFeatureSlice plus expectLoadingState/expectErrorState/expectEmptyState/waitForState) that asserts the data-contract-state markers the shared state primitives render, and teach verify:features to require each slice's tests to exercise the loading, error and empty states. Existing slices are flagged in a migration ledger; the checker itself is covered by good/bad fixture slices. Refs Parcel-Protocol#12
|
@JamesHunt22 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
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.
Overview
Extends
npm run verify:featuresso it checks a slice's runtime contract, not only its file layout. Today the checker answers "is there an__tests__/directory with at least five files?" — a slice whose tests never touch the loading, error or empty state passes anyway. This adds a shared harness incore/testing/and makesverify:featuresfail when a slice's tests do not drive those states, so "has a test file" and "the test file exercises the states" are no longer the same answer.Related Issue
#12
Changes
Shared runtime contract harness
core/testing/contract.tsxrenderFeatureSlice(id, ui, options)mounts a slice panel inside the real providers and returns contract assertions bound to the render result.expectLoadingState(),expectErrorState(),expectEmptyState(), plus the genericexpectState(state)and asyncwaitForState(state).data-contract-stateattribute rather than on copy, so the same API works for a lookup form, a QR generator and a wallet-connect flow. The role/aria-busyfallbacks cover slices that hand-roll an indicator.core/testing/index.ts— re-exports the harness alongsiderenderFeature.Shared state markers
core/ui/EmptyState.tsx— rendersdata-contract-state="empty".core/ui/StatusMessage.tsx— rendersdata-contract-state={type}.core/ui/Skeleton.tsx—SkeletonandSkeletonRowsrenderdata-contract-state="loading".Checker
scripts/verify-features.mjs__tests__/and requiresrenderFeatureSliceplus a call for each of loading / error / empty (the named helper or theexpectState("…")/waitForState("…")form). Failures name the missing state.CONTRACT_STATES,collectTestSources,evaluateContractCoverage,checkContractCoverage, and addsfeaturesDir/contractPendingoverrides toverifySliceso the rule is testable without touching the realfeatures/directory.scripts/verify-features.d.mts— types for the.mjsso the checker test compiles under strict TypeScript.scripts/feature-contract-migration.json— the 53 existing slices whose tests still assert the states by hand. They are reported as a warning, not a failure, while the migration is in flight; a slice that is not in the ledger must satisfy the contract, so CI fails clearly for any new slice that skips a state.Checker test and fixtures
core/__tests__/verifyFeaturesContract.test.ts— covers the good and bad fixture slices plus a partially-migrated case that must report exactly the skipped state.scripts/__tests__/fixtures/contract-good-slice/**andscripts/__tests__/fixtures/contract-bad-slice/**— deliberately minimal fixture slices, read as source by the checker (they are outside vitest'sincludeglobs and are never executed).Documentation
docs/FEATURE_CONTRACT.md— new section 11 documents the harness, thedata-contract-stateconvention, the checker and the migration ledger, and the "done" checklist gains a harness line.Verification Results
Not run — this change was authored entirely through the GitHub REST API with no local checkout of the repository, so no
npm runcommand could be executed. A reviewer must runnpm run check(or let the existing CI workflow runnpm run verify:features,npm run testandnpm run build).What was actually executed, in isolation, against the exported checker logic:
The repo's own
npm run test,npm run verify:featuresandnpm run buildwere not run here. The checker logic above is the same code path the new vitest test exercises, but the vitest suite itself has not been executed.verify:featuresdistinguishes 'has a test file' from 'test file exercises loading/error/empty states'FEATURE_CONTRACT.mdcore/testing/contract.tsx, documented indocs/FEATURE_CONTRACT.mdsection 11.scripts/feature-contract-migration.json. No existing test was rewritten blind and no migration was claimed without running it.verify:featureswith__tests__/ — runtime contract: …; CI already runs that step.core/__tests__/verifyFeaturesContract.test.tsruns the checker overscripts/__tests__/fixtures/contract-good-sliceandcontract-bad-slice.Closes #12