test(web): add docs nav route consistency tests#187
Conversation
|
@PranavAgarkar07 is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe test suite for ChangesDocs Navigation Filesystem Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@PranavAgarkar07 Which issue does it solves please mention that in the description? |
|
@codex review |
AnkanMisra
left a comment
AnkanMisra
left a comment
There was a problem hiding this comment.
resolve conflicts and fix what bugs codex found and link issue
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a06e83521
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { existsSync } from "node:fs"; | ||
| import { join, resolve, dirname } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { DOCS_NAV_ITEMS } from "./docs-nav"; |
There was a problem hiding this comment.
In this commit's tree, the only files under web/src/components/docs are the two new tests, and there is no web/src/app/docs directory either, so this import cannot resolve and docs-nav.test.ts fails before any route assertions run. In any fresh checkout that runs the claimed bunx vitest run or the repo's bun run test type check, this test file is broken; add the actual docs nav/pages first or point the test at existing code.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,16 @@ | |||
| import { describe, expect, test } from "vitest"; | |||
| import { extractCopyText } from "./copy-code-block"; | |||
There was a problem hiding this comment.
There is no web/src/components/docs/copy-code-block implementation in this commit's tree, so this new test imports a module that does not exist and fails module resolution before exercising extractCopyText. This makes the documented Vitest run and the web TypeScript check fail on a clean checkout; include the component/helper being tested or remove/retarget this test until that code exists.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,54 @@ | |||
| import { describe, expect, it } from "vitest"; | |||
There was a problem hiding this comment.
These new files import vitest, but neither web/package.json nor the Bun lockfiles declare Vitest, and the web tsconfig.json includes **/*.ts/**/*.tsx. On a clean install, bun run test now has to resolve this package and will fail with a missing-module error even before considering the assertions; add Vitest as a web devDependency and update the lockfile/test script rather than relying on an unpinned bunx download.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| describe("docs navigation", () => { |
There was a problem hiding this comment.
This suite is added under web/**, but the inspected web workflow only runs bun run lint and bun run build, while web/package.json's test script is just tsc --noEmit; no checked command runs these Vitest assertions. Even after the missing modules/dependency are fixed, docs navigation regressions can merge unnoticed unless the web validation script or .github/workflows/web-lint-build.yml invokes Vitest.
Useful? React with 👍 / 👎.
| it("every nav link points to an existing docs page file", () => { | ||
| const missing = DOCS_NAV_ITEMS.filter((item) => !pageExists(item.href)); |
There was a problem hiding this comment.
The new route-consistency suite only checks a hard-coded nav list and verifies each nav entry has a page; it never enumerates src/app/docs to fail when someone adds a docs page without adding it to DOCS_NAV_ITEMS. That leaves the advertised “no orphaned docs pages” coverage absent, so add a reverse check over the docs page files once the docs directory/module exists.
Useful? React with 👍 / 👎.
Validates that every DOCS_NAV_ITEMS href maps to an existing page file in src/app/docs/. Also verifies no docs page exists without a nav entry (reverse orphan check) and that routes/labels are unique. Keeps project convention of using bun:test instead of vitest. Fixes AnkanMisra#176 Co-authored-by: codex <codex@users.noreply.github.com>
PranavAgarkar07
force-pushed
the
test/docs-nav-routes
branch
from
1a06e83 to
a58813c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/docs/docs-nav.test.ts (1)
23-35: ⚡ Quick winOrphan-page detection is shallow and can miss nested docs routes.
discoverDocsPageSlugs()only inspects immediate children ofsrc/app/docs. If a page exists deeper (for examplesrc/app/docs/guides/setup/page.mdx), this test won’t detect it as orphaned.Suggested recursive scan
function discoverDocsPageSlugs(): string[] { - const slugs: string[] = []; - const entries = readdirSync(DOCS_PAGES_DIR, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory()) continue; - const dirPath = join(DOCS_PAGES_DIR, entry.name); - const hasPage = PAGE_EXTENSIONS.some((ext) => - existsSync(join(dirPath, `page${ext}`)), - ); - if (hasPage) slugs.push(`/docs/${entry.name}`); - } - return slugs.sort(); + const slugs: string[] = []; + + function walk(currentDir: string, routeParts: string[]) { + const hasPage = PAGE_EXTENSIONS.some((ext) => + existsSync(join(currentDir, `page${ext}`)), + ); + if (hasPage && routeParts.length > 0) { + slugs.push(`/docs/${routeParts.join("/")}`); + } + + const entries = readdirSync(currentDir, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) continue; + walk(join(currentDir, entry.name), [...routeParts, entry.name]); + } + } + + walk(DOCS_PAGES_DIR, []); + return slugs.sort(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/docs/docs-nav.test.ts` around lines 23 - 35, discoverDocsPageSlugs currently only checks immediate children of DOCS_PAGES_DIR and misses nested routes; update the function to recursively walk the DOCS_PAGES_DIR (using readdirSync with { withFileTypes: true } or a small recursive helper) and for each directory check for a page file using PAGE_EXTENSIONS, then push the slug built from the directory path relative to DOCS_PAGES_DIR (e.g. `/docs/${relativePath}`) instead of only entry.name; keep usage of DOCS_PAGES_DIR, PAGE_EXTENSIONS and the discoverDocsPageSlugs function names to locate and replace the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/src/components/docs/docs-nav.test.ts`:
- Around line 23-35: discoverDocsPageSlugs currently only checks immediate
children of DOCS_PAGES_DIR and misses nested routes; update the function to
recursively walk the DOCS_PAGES_DIR (using readdirSync with { withFileTypes:
true } or a small recursive helper) and for each directory check for a page file
using PAGE_EXTENSIONS, then push the slug built from the directory path relative
to DOCS_PAGES_DIR (e.g. `/docs/${relativePath}`) instead of only entry.name;
keep usage of DOCS_PAGES_DIR, PAGE_EXTENSIONS and the discoverDocsPageSlugs
function names to locate and replace the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 677efb70-4ff5-4ace-a10d-5fcba6e847eb
📒 Files selected for processing (1)
web/src/components/docs/docs-nav.test.ts
2 participants
Summary
Adds a focused test in
web/src/components/docs/docs-nav.test.tsthat validates every entry inDOCS_NAV_ITEMSmaps to an existing page file undersrc/app/docs/. Three test cases:hrefresolves to a real file (.mdx,.tsx,.ts,.jsx,.js)Also fixes a pre-existing import bug:
copy-code-block.test.tsximported from"bun:test"which is unavailable in the vitest runner. Changed to"vitest"to match the project convention.Type Of Change
Affected Areas
web/)Contributor Checklist
Verification
Notes For Reviewers
The test uses
node:fsto check file existence without a running dev server. ThepageExistshelper normalises next.js App Router paths ("/docs"→"docs/page","/docs/quickstart"→"docs/quickstart/page").Summary by CodeRabbit