Skip to content

Commit cd74508

Browse files
committed
test: add workspaceUtils utility and refactor test paths
- Introduced `workspaceUtils` for consistent workspace directory resolution - Added unit tests for `findWorkspaceRoot`, `getScriptPath`, `getWorkspacePath`, and `setupTestPaths` - Refactored CLI and script integration tests to utilize `workspaceUtils` - Updated `nx.json` and `package.json` with explicit `test:unit` and `test:integration` targets for granular control
1 parent 071ea22 commit cd74508

File tree

8 files changed

+322
-8
lines changed

8 files changed

+322
-8
lines changed

CLAUDE.md

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ The project uses a 5-group testing strategy optimized for efficiency and cost ma
5656
#### Group 3: Pre-Commit Tests (Git Hook - Fast Feedback)
5757
- **Automatic**: Runs on `git commit`
5858
- **Manual**: `pnpm test:pre-commit`
59-
- **What runs**: `nx affected --targets=typecheck,lint,test --parallel=3` + config validation
59+
- **What runs**: `nx affected --targets=typecheck,lint,test:unit --parallel=3` + config validation
6060
- **Target**: < 60 seconds
6161

6262
#### Group 4: Pre-Push Tests (Git Hook - Deployment Focus)
@@ -144,9 +144,104 @@ standards-dev/
144144
- **Pre-commit**: TypeScript, ESLint, unit tests run automatically
145145
- **Pre-push**: Build regression tests (branch-aware) run automatically
146146
- **Test runner**: Vitest for unit/integration tests
147-
- **E2E testing**: Puppeteer for interface testing when needed
147+
- **E2E testing**: Playwright for interface testing when needed
148148
- **Always test before commits**: Tests must pass before offering to commit
149149

150+
## Test Placement Decision Tree (CRITICAL - Reference When Creating Tests)
151+
152+
**ALWAYS use this decision tree when creating new tests to ensure they run in the correct scenario:**
153+
154+
### 🎯 **Where to Put New Tests**
155+
156+
```
157+
📁 Test Placement Decision Tree
158+
159+
├── 🔧 **Unit Tests** (Pure logic, no external deps)
160+
│ └── 📍 Location: `packages/theme/src/tests/components/ComponentName.test.tsx`
161+
│ └── 🎯 Target: Runs in `test:unit` (pre-commit) + `test` (comprehensive)
162+
│ └── ✅ Examples: Component rendering, pure functions, config parsing
163+
164+
├── 🔗 **Integration Tests** (External APIs, file system, CLI tools)
165+
│ ├── 📍 Scripts: `packages/theme/src/tests/scripts/`
166+
│ ├── 📍 External Services: `packages/theme/src/tests/deployment/`
167+
│ └── 🎯 Target: Only runs in `test:integration` (comprehensive testing)
168+
│ └── ✅ Examples: Google Sheets API, file operations, CLI execution
169+
170+
├── 🌐 **E2E Tests** (Browser automation, full user workflows)
171+
│ ├── 📍 Workspace: `/e2e/site-validation.spec.ts`
172+
│ ├── 📍 Site-specific: `standards/SITE/e2e/feature.e2e.test.ts`
173+
│ └── 🎯 Target: `nx run standards-dev:e2e` or `nx run SITE:e2e`
174+
│ └── ✅ Examples: Navigation, form submission, visual checks
175+
176+
└── 🎯 **Performance/Visual** (Screenshots, load testing)
177+
└── 📍 Location: `/e2e/visual-regression-enhanced.spec.ts`
178+
└── 🎯 Target: Specialized E2E runs
179+
```
180+
181+
### 🚦 **Quick Decision Questions**
182+
183+
**Ask yourself when creating a test:**
184+
185+
1. **Does it make network calls or use external APIs?** → Integration test
186+
2. **Does it render in a browser or test user flows?** → E2E test
187+
3. **Does it test component logic in isolation?** → Unit test
188+
4. **Does it validate file system operations?** → Integration test
189+
5. **Does it test configuration parsing only?** → Unit test
190+
191+
### 📋 **Test Coverage by Scenario**
192+
193+
| Scenario | Unit Tests | Integration Tests | E2E Tests | When It Runs |
194+
|----------|------------|-------------------|-----------|--------------|
195+
| **Pre-commit** | ✅ Fast feedback | ❌ Too slow | ❌ Too slow | Every `git commit` |
196+
| **Pre-push** |||| Every `git push` |
197+
| **Comprehensive** |||| Manual/CI full runs |
198+
| **Affected** | ✅ Only changed | ✅ Only changed || Development workflow |
199+
| **CI** |||| Production deployments |
200+
201+
### 🎪 **Examples of Correct Placement**
202+
203+
```typescript
204+
// ✅ Unit Test - packages/theme/src/tests/components/
205+
describe('VocabularyTable', () => {
206+
it('renders headers correctly', () => {
207+
// Tests pure component logic
208+
});
209+
});
210+
211+
// ✅ Integration Test - packages/theme/src/tests/scripts/
212+
describe('Google Sheets API', () => {
213+
it('fetches vocabulary data', async () => {
214+
// Tests external API integration
215+
});
216+
});
217+
218+
// ✅ E2E Test - /e2e/
219+
test('user can navigate between sites', async ({ page }) => {
220+
// Tests full browser workflows
221+
});
222+
```
223+
224+
**Key principle**: Unit tests run fast in pre-commit for immediate feedback, integration tests run in comprehensive scenarios, E2E tests validate complete user experiences.
225+
226+
### 🛠️ **Test Utilities**
227+
228+
For consistent directory handling in tests, use the `workspaceUtils` helper:
229+
230+
```typescript
231+
import { findWorkspaceRoot, getScriptPath, setupTestPaths } from '../utils/workspaceUtils';
232+
233+
// Get workspace root reliably (instead of process.cwd())
234+
const workspaceRoot = findWorkspaceRoot();
235+
236+
// Get path to scripts
237+
const scriptPath = getScriptPath('vocabulary-comparison.mjs');
238+
239+
// Get common test paths
240+
const { workspaceRoot, scriptsDir, tmpDir, packagesDir, themeDir } = setupTestPaths();
241+
```
242+
243+
**Always use `workspaceUtils` instead of `process.cwd()` in integration tests** to ensure consistent directory resolution regardless of test execution context.
244+
150245
### Content and Navigation Rules
151246
- **Never hardcode URLs**: Always use SiteLink component or configuration-based URLs
152247
- **Broken links categorization**:

nx.json

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,30 @@
100100
"{projectRoot}/e2e/**/*",
101101
"{projectRoot}/**/*.e2e.{js,ts,jsx,tsx}"
102102
]
103+
},
104+
"test:unit": {
105+
"cache": true,
106+
"outputs": ["{projectRoot}/coverage", "{projectRoot}/test-results"],
107+
"inputs": [
108+
"default",
109+
"{projectRoot}/test/**/*",
110+
"{projectRoot}/**/*.test.{js,ts,jsx,tsx}",
111+
"{projectRoot}/**/*.spec.{js,ts,jsx,tsx}",
112+
"{workspaceRoot}/vite.config.ts",
113+
"{workspaceRoot}/vitest.config.*"
114+
]
115+
},
116+
"test:integration": {
117+
"cache": true,
118+
"outputs": ["{projectRoot}/coverage", "{projectRoot}/test-results"],
119+
"inputs": [
120+
"default",
121+
"{projectRoot}/test/**/*",
122+
"{projectRoot}/**/*.test.{js,ts,jsx,tsx}",
123+
"{projectRoot}/**/*.spec.{js,ts,jsx,tsx}",
124+
"{workspaceRoot}/vite.config.ts",
125+
"{workspaceRoot}/vitest.config.*"
126+
]
103127
}
104128
},
105129
"plugins": [

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@
137137
"test:full": "pnpm typecheck && pnpm lint --quiet && pnpm test && pnpm test:builds:config",
138138
"test:regression": "nx run standards-dev:regression:full",
139139
"test:regression:nx:fast": "nx run standards-dev:regression:fast",
140-
"test:pre-commit": "nx affected --targets=typecheck,lint,test --parallel=3 && node scripts/test-site-builds-affected.js --env local --skip-build",
140+
"test:pre-commit": "nx affected --targets=typecheck,lint,test:unit --parallel=3 && node scripts/test-site-builds-affected.js --env local --skip-build",
141141
"test:pre-push": "pnpm test:pre-commit && pnpm test:builds:critical",
142142
"test:pre-push:nx": "nx affected --target=lint && nx affected --target=typecheck && nx affected --target=test && nx affected --target=build",
143143
"test:e2e": "nx run standards-dev:e2e",

packages/theme/project.json

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,45 @@
3636
"{workspaceRoot}/vitest.config.*"
3737
]
3838
},
39+
"test:unit": {
40+
"executor": "@nx/vite:test",
41+
"options": {
42+
"config": "{workspaceRoot}/vite.config.ts",
43+
"exclude": [
44+
"**/scripts/**",
45+
"**/deployment/**"
46+
]
47+
},
48+
"outputs": ["{projectRoot}/coverage"],
49+
"cache": true,
50+
"inputs": [
51+
"default",
52+
"{projectRoot}/src/**/*.{test,spec}.{js,ts,jsx,tsx}",
53+
"!{projectRoot}/src/tests/scripts/**/*",
54+
"!{projectRoot}/src/tests/deployment/**/*",
55+
"{workspaceRoot}/vite.config.ts",
56+
"{workspaceRoot}/vitest.config.*"
57+
]
58+
},
59+
"test:integration": {
60+
"executor": "@nx/vite:test",
61+
"options": {
62+
"config": "{workspaceRoot}/vite.config.ts",
63+
"include": [
64+
"**/scripts/**",
65+
"**/deployment/**"
66+
]
67+
},
68+
"outputs": ["{projectRoot}/coverage"],
69+
"cache": true,
70+
"inputs": [
71+
"default",
72+
"{projectRoot}/src/tests/scripts/**/*",
73+
"{projectRoot}/src/tests/deployment/**/*",
74+
"{workspaceRoot}/vite.config.ts",
75+
"{workspaceRoot}/vitest.config.*"
76+
]
77+
},
3978
"typecheck": {
4079
"executor": "nx:run-commands",
4180
"options": {

packages/theme/src/tests/scripts/check-mediatype-languages.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { exec } from 'child_process';
33
import { promisify } from 'util';
44
import path from 'path';
55
import fs from 'fs/promises';
6+
import { getScriptPath, setupTestPaths } from '../utils/workspaceUtils';
67

78
const execAsync = promisify(exec);
89

@@ -14,9 +15,8 @@ const mockEnv = {
1415
};
1516

1617
describe('check-mediatype-languages.mjs', () => {
17-
const projectRoot = path.resolve(__dirname, '../../../../..');
18-
const scriptPath = path.join(projectRoot, 'scripts', 'check-mediatype-languages.mjs');
19-
const tmpDir = path.join(projectRoot, 'tmp');
18+
const scriptPath = getScriptPath('check-mediatype-languages.mjs');
19+
const { workspaceRoot, tmpDir } = setupTestPaths();
2020

2121
beforeEach(async () => {
2222
// Debug: Check current working directory

packages/theme/src/tests/scripts/vocabulary-comparison-cli.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
66
import { exec } from 'child_process';
77
import { promisify } from 'util';
8-
import path from 'path';
8+
import { getScriptPath, setupTestPaths } from '../utils/workspaceUtils';
99

1010
const execAsync = promisify(exec);
1111

1212
describe('Vocabulary Comparison CLI', () => {
13-
const scriptPath = path.join(process.cwd(), '../../scripts/vocabulary-comparison.mjs');
13+
const scriptPath = getScriptPath('vocabulary-comparison.mjs');
14+
const { workspaceRoot } = setupTestPaths();
1415

1516
// Skip these tests in CI environment
1617
if (process.env.CI) {
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { describe, it, expect } from 'vitest';
2+
import path from 'path';
3+
import fs from 'fs';
4+
import { findWorkspaceRoot, getScriptPath, getWorkspacePath, setupTestPaths } from './workspaceUtils';
5+
6+
describe('workspaceUtils', () => {
7+
describe('findWorkspaceRoot', () => {
8+
it('should find the workspace root directory', () => {
9+
const workspaceRoot = findWorkspaceRoot();
10+
11+
// Should be an absolute path
12+
expect(path.isAbsolute(workspaceRoot)).toBe(true);
13+
14+
// Should contain a package.json with the correct name
15+
const packageJsonPath = path.join(workspaceRoot, 'package.json');
16+
expect(fs.existsSync(packageJsonPath)).toBe(true);
17+
18+
const packageJson = require(packageJsonPath);
19+
expect(packageJson.name).toBe('standards-dev');
20+
});
21+
22+
it('should return the same path when called multiple times', () => {
23+
const path1 = findWorkspaceRoot();
24+
const path2 = findWorkspaceRoot();
25+
26+
expect(path1).toBe(path2);
27+
});
28+
});
29+
30+
describe('getScriptPath', () => {
31+
it('should return correct path for script files', () => {
32+
const scriptPath = getScriptPath('vocabulary-comparison.mjs');
33+
34+
expect(path.isAbsolute(scriptPath)).toBe(true);
35+
expect(scriptPath).toContain('scripts');
36+
expect(scriptPath).toContain('vocabulary-comparison.mjs');
37+
});
38+
});
39+
40+
describe('getWorkspacePath', () => {
41+
it('should return correct path for relative workspace paths', () => {
42+
const tmpPath = getWorkspacePath('tmp');
43+
const packagesPath = getWorkspacePath('packages/theme');
44+
45+
expect(path.isAbsolute(tmpPath)).toBe(true);
46+
expect(path.isAbsolute(packagesPath)).toBe(true);
47+
expect(tmpPath).toContain('tmp');
48+
expect(packagesPath).toContain('packages');
49+
expect(packagesPath).toContain('theme');
50+
});
51+
});
52+
53+
describe('setupTestPaths', () => {
54+
it('should return all common test paths', () => {
55+
const paths = setupTestPaths();
56+
57+
expect(paths).toHaveProperty('workspaceRoot');
58+
expect(paths).toHaveProperty('scriptsDir');
59+
expect(paths).toHaveProperty('tmpDir');
60+
expect(paths).toHaveProperty('packagesDir');
61+
expect(paths).toHaveProperty('themeDir');
62+
63+
// All paths should be absolute
64+
Object.values(paths).forEach(pathValue => {
65+
expect(path.isAbsolute(pathValue)).toBe(true);
66+
});
67+
});
68+
69+
it('should return paths that exist in the workspace', () => {
70+
const { workspaceRoot, scriptsDir, packagesDir } = setupTestPaths();
71+
72+
// These directories should exist
73+
expect(fs.existsSync(workspaceRoot)).toBe(true);
74+
expect(fs.existsSync(scriptsDir)).toBe(true);
75+
expect(fs.existsSync(packagesDir)).toBe(true);
76+
});
77+
});
78+
});
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* Utility functions for test workspace management
3+
* Provides consistent directory handling across all test files
4+
*/
5+
6+
import path from 'path';
7+
import fs from 'fs';
8+
9+
/**
10+
* Finds the workspace root by looking for package.json with "name": "standards-dev"
11+
* This is more reliable than process.cwd() which can vary depending on test execution context
12+
*
13+
* @returns {string} Absolute path to the workspace root
14+
* @throws {Error} If workspace root cannot be found
15+
*/
16+
export const findWorkspaceRoot = (): string => {
17+
let currentDir = __dirname;
18+
19+
while (currentDir !== path.dirname(currentDir)) {
20+
const packageJsonPath = path.join(currentDir, 'package.json');
21+
22+
if (fs.existsSync(packageJsonPath)) {
23+
try {
24+
const packageJson = require(packageJsonPath);
25+
if (packageJson.name === 'standards-dev') {
26+
return currentDir;
27+
}
28+
} catch (e) {
29+
// Continue searching if package.json is malformed
30+
}
31+
}
32+
33+
currentDir = path.dirname(currentDir);
34+
}
35+
36+
throw new Error('Could not find workspace root - no package.json with name "standards-dev" found');
37+
};
38+
39+
/**
40+
* Gets the absolute path to a script in the workspace scripts directory
41+
*
42+
* @param scriptName - Name of the script file (e.g., 'vocabulary-comparison.mjs')
43+
* @returns {string} Absolute path to the script
44+
*/
45+
export const getScriptPath = (scriptName: string): string => {
46+
const workspaceRoot = findWorkspaceRoot();
47+
return path.join(workspaceRoot, 'scripts', scriptName);
48+
};
49+
50+
/**
51+
* Gets the absolute path to a directory relative to workspace root
52+
*
53+
* @param relativePath - Path relative to workspace root (e.g., 'tmp', 'packages/theme')
54+
* @returns {string} Absolute path to the directory
55+
*/
56+
export const getWorkspacePath = (relativePath: string): string => {
57+
const workspaceRoot = findWorkspaceRoot();
58+
return path.join(workspaceRoot, relativePath);
59+
};
60+
61+
/**
62+
* Sets up common test environment paths
63+
* Returns commonly used paths for integration tests
64+
*
65+
* @returns {object} Object containing commonly used paths
66+
*/
67+
export const setupTestPaths = () => {
68+
const workspaceRoot = findWorkspaceRoot();
69+
70+
return {
71+
workspaceRoot,
72+
scriptsDir: path.join(workspaceRoot, 'scripts'),
73+
tmpDir: path.join(workspaceRoot, 'tmp'),
74+
packagesDir: path.join(workspaceRoot, 'packages'),
75+
themeDir: path.join(workspaceRoot, 'packages', 'theme')
76+
};
77+
};

0 commit comments

Comments
 (0)