Skip to content

Commit e583162

Browse files
committed
Continue fixing tests to make them work on windows
1 parent a9f2d80 commit e583162

File tree

4 files changed

+117
-27
lines changed

4 files changed

+117
-27
lines changed

docs/src/modules/4-advanced/testing.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,62 @@ test('CurveTab', () => {
4545

4646
For more instructions on how to write tests you can refer to the [Vitest website](https://vitest.dev/guide/#writing-tests).
4747

48+
### Test Filtering
49+
50+
> [!INFO]
51+
> More information on this entire section can be found [here](https://vitest.dev/guide/filtering.html#skipping-suites-and-tests);
52+
53+
While writing tests, you might find that you might want to focus on a single test or single group of tests. For this, `vitest` provides on its `test`, `it` and `describe` functions
54+
a `.skip` and a `.only` property:
55+
```ts
56+
describe('Test1 and Test3 will run!', () => {
57+
test('Test1', () => {});
58+
test.skip('Test2', () => {});
59+
test('Test3', () => {})
60+
});
61+
```
62+
You don't have to comment out your tests; simply use `.skip` to indicate that a test block should not be executed.
63+
64+
If for some reason you want to skip your tests based on some condition, `vitest` provides the `skipIf` property:
65+
```ts
66+
describe('Test1 and Test3 will run!', () => {
67+
test('Test1', () => {});
68+
test.skipIf(condition)('Test2', () => {});
69+
test('Test3', () => {})
70+
});
71+
```
72+
73+
`.only` is kind of the reverse of `.skip`. Tests that you use `.only` with will be the **only** tests that run in that file:
74+
```ts
75+
describe('Only Test 2 will run!', () => {
76+
test('Test1', () => {});
77+
test.only('Test2', () => {});
78+
test('Test3', () => {})
79+
});
80+
```
81+
The main runner that runs unit tests on the CI/CD pipeline does not allow for `.only`. You can simulate this behaviour by running your tests with the
82+
`--no-allow-only` flag. This behaviour is intended to prevent you from causing only part of your tests to run.
83+
84+
> [!INFO] Linting
85+
> There is an ESLint rule configured to warn you if you use focused tests (using `.only`). This is not an issue, so long as you remember
86+
> to remove `.only` before pushing to the main repository.
87+
88+
Pushing with skipped tests however, is allowed. Do leave a comment explaining why the test is skipped:
89+
```ts
90+
// Test is skipped because there is an outstanding bug
91+
test.skip('Test path resolution', () => {})
92+
```
93+
94+
### Stubbing Tests
95+
If you want to indicate that tests should be written for certain functionality in the future, you can use `.todo`:
96+
97+
```ts
98+
// Needs to be implemented when the outstanding bug is fixed
99+
test.todo('Test path resolution')
100+
```
101+
102+
TODO tests still generate an entry in your test reports, but will be considered neither failed nor passed.
103+
48104
## Running Tests
49105
To run tests for a given bundle or tab, simply run either of the following commands within the directory:
50106
```sh
@@ -56,6 +112,8 @@ By default, `vitest` will quit after running tests. If you wish to run the tests
56112

57113
You can also use `--update` to update snapshots and `--coverage` to run the V8 coverage reporter.
58114

115+
For bundles and tabs, the test environment should always be `jsdom`, since they are intended for the browser.
116+
59117
## Integration with Git Hooks
60118
Any tests that you have written must be pass in order for you to push to the main repository, as well as for your pull requests to be merged.
61119

lib/buildtools/src/testing/__tests__/utils.test.ts

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import type { Dirent } from 'fs';
22
import fs from 'fs/promises';
3+
import pathlib from 'path';
4+
import { bundlesDir, tabsDir } from '@sourceacademy/modules-repotools/getGitRoot';
35
import * as manifest from '@sourceacademy/modules-repotools/manifest';
46
import * as configs from '@sourceacademy/modules-repotools/testing';
57
import { beforeEach, describe, expect, it, vi } from 'vitest';
68
import type { TestProjectInlineConfiguration } from 'vitest/config';
9+
import { testMocksDir } from '../../__tests__/fixtures.js';
710
import * as utils from '../utils.js';
811

912
class ENOENT extends Error {
@@ -138,6 +141,8 @@ describe('Test setBrowserOptions', () => {
138141

139142
describe('Test getTestConfiguration', () => {
140143
describe('With tabs', () => {
144+
const tabPath = pathlib.join(tabsDir, 'Tab0');
145+
141146
beforeEach(() => {
142147
mockedReadFile.mockImplementation(p => {
143148
if (p === '/tabs/Tab0/package.json') {
@@ -152,51 +157,54 @@ describe('Test getTestConfiguration', () => {
152157
vi.spyOn(manifest, 'resolveSingleTab').mockResolvedValueOnce({
153158
type: 'tab',
154159
name: 'Tab0',
155-
directory: '/tabs/Tab0',
156-
entryPoint: '/tabs/Tab0/index.tsx'
160+
directory: tabPath,
161+
entryPoint: pathlib.join(tabPath, 'index.tsx')
157162
});
158163
});
159164

160165
it('Should return the config if the tab has tests', async () => {
161166
mockHasTestsOnce(true);
162-
await expect(utils.getTestConfiguration('/tabs/Tab0', false)).resolves.toMatchObject({
167+
await expect(utils.getTestConfiguration(tabPath, false)).resolves.toMatchObject({
163168
severity: 'success',
164169
config: {
165170
...configs.sharedTabsConfig,
166171
test: {
167172
...configs.sharedTabsConfig.test!,
168173
name: 'Tab0 Tab',
169-
root: '/tabs/Tab0'
174+
root: tabPath
170175
}
171176
}
172177
});
173178
});
174179

175180
it('Should return the config even if the function was called from not the tab\'s root directory', async () => {
181+
const subDir = pathlib.join(tabPath, 'sub', 'directory');
176182
mockHasTestsOnce(true);
177-
await expect(utils.getTestConfiguration('/tabs/Tab0/sub/directory', false)).resolves.toMatchObject({
183+
await expect(utils.getTestConfiguration(subDir, false)).resolves.toMatchObject({
178184
severity: 'success',
179185
config: {
180186
...configs.sharedTabsConfig,
181187
test: {
182188
...configs.sharedTabsConfig.test!,
183189
name: 'Tab0 Tab',
184-
root: '/tabs/Tab0'
190+
root: tabPath
185191
}
186192
}
187193
});
188194
});
189195

190196
it('Should return null if the tab has no tests', async () => {
191197
mockHasTestsOnce(false);
192-
await expect(utils.getTestConfiguration('/tabs/Tab0', false)).resolves.toMatchObject({
198+
await expect(utils.getTestConfiguration(tabPath, false)).resolves.toMatchObject({
193199
severity: 'success',
194200
config: null
195201
});
196202
});
197203
});
198204

199205
describe('With bundles', () => {
206+
const bundlePath = pathlib.join(bundlesDir, 'bundle0');
207+
200208
beforeEach(() => {
201209
mockedReadFile.mockImplementation(p => {
202210
if (p === '/bundles/bundle0/package.json') {
@@ -214,54 +222,57 @@ describe('Test getTestConfiguration', () => {
214222
type: 'bundle',
215223
name: 'bundle0',
216224
manifest: {},
217-
directory: '/bundles/bundle0',
225+
directory: bundlePath
218226
}
219227
});
220228
});
221229

222230
it('Should return the config if the bundle has tests', async () => {
223231
mockHasTestsOnce(true);
224-
await expect(utils.getTestConfiguration('/bundles/bundle0', false)).resolves.toMatchObject({
232+
await expect(utils.getTestConfiguration(bundlePath, false)).resolves.toMatchObject({
225233
severity: 'success',
226234
config: {
227235
...configs.baseVitestConfig,
228236
test: {
229237
...configs.baseVitestConfig.test!,
230238
name: 'bundle0 Bundle',
231-
root: '/bundles/bundle0'
239+
root: bundlePath
232240
}
233241
}
234242
});
235243
});
236244

237245
it('Should return the config even if the function was called from not the tab\'s root directory', async () => {
246+
const subdir = pathlib.join(bundlePath, 'sub', 'directory');
238247
mockHasTestsOnce(true);
239-
await expect(utils.getTestConfiguration('/bundles/bundle0/sub/directory', false)).resolves.toMatchObject({
248+
await expect(utils.getTestConfiguration(subdir, false)).resolves.toMatchObject({
240249
severity: 'success',
241250
config: {
242251
...configs.baseVitestConfig,
243252
test: {
244253
...configs.baseVitestConfig.test!,
245254
name: 'bundle0 Bundle',
246-
root: '/bundles/bundle0'
255+
root: bundlePath
247256
}
248257
}
249258
});
250259
});
251260

252261
it('Should return null if the bundle has no tests', async () => {
253262
mockHasTestsOnce(false);
254-
await expect(utils.getTestConfiguration('/bundles/bundle0', false)).resolves.toMatchObject({
263+
await expect(utils.getTestConfiguration(bundlePath, false)).resolves.toMatchObject({
255264
severity: 'success',
256265
config: null
257266
});
258267
});
259268
});
260269

261270
describe('With neither', () => {
271+
const libPath = pathlib.join(testMocksDir, 'dir');
272+
262273
beforeEach(() => {
263274
mockedReadFile.mockImplementation(p => {
264-
if (p === '/dir/package.json') {
275+
if (p === pathlib.join(libPath, 'package.json')) {
265276
return Promise.resolve(JSON.stringify({
266277
name: "@sourceacademy/a-pacakge"
267278
}));
@@ -278,14 +289,35 @@ describe('Test getTestConfiguration', () => {
278289
name: 'Test0'
279290
}
280291
});
281-
await expect(utils.getTestConfiguration('/dir', false)).resolves.toMatchObject({
292+
await expect(utils.getTestConfiguration(libPath, false)).resolves.toMatchObject({
293+
severity: 'success',
294+
config: {
295+
...configs.baseVitestConfig,
296+
test: {
297+
...configs.baseVitestConfig.test!,
298+
name: 'Test0',
299+
root: libPath
300+
}
301+
}
302+
});
303+
});
304+
305+
it('should return the package directory even if run from a sub directory', async () => {
306+
mockHasTestsOnce(true);
307+
mockedLoadConfig.mockResolvedValueOnce({
308+
test: {
309+
name: 'Test0'
310+
}
311+
});
312+
const subdir = pathlib.join(libPath, 'sub', 'directory');
313+
await expect(utils.getTestConfiguration(subdir, false)).resolves.toMatchObject({
282314
severity: 'success',
283315
config: {
284316
...configs.baseVitestConfig,
285317
test: {
286318
...configs.baseVitestConfig.test!,
287319
name: 'Test0',
288-
root: '/dir'
320+
root: libPath
289321
}
290322
}
291323
});
@@ -295,19 +327,19 @@ describe('Test getTestConfiguration', () => {
295327
mockedLoadConfig.mockResolvedValueOnce(null);
296328
mockHasTestsOnce(true);
297329

298-
await expect(utils.getTestConfiguration('/dir', false))
330+
await expect(utils.getTestConfiguration(libPath, false))
299331
.resolves
300332
.toMatchObject({
301333
severity: 'error',
302-
errors: ['Tests were found for /dir, but no vitest config could be located']
334+
errors: [`Tests were found for ${libPath}, but no vitest config could be located`]
303335
});
304336
});
305337

306338
it('should not return an error if the directory has no vitest config or tests', async () => {
307339
mockedLoadConfig.mockResolvedValueOnce(null);
308340
mockHasTestsOnce(false);
309341

310-
await expect(utils.getTestConfiguration('/dir', false))
342+
await expect(utils.getTestConfiguration(libPath, false))
311343
.resolves
312344
.toMatchObject({
313345
severity: 'success',
@@ -430,6 +462,7 @@ describe('Test getAllTestConfigurations', () => {
430462
}
431463
}
432464
});
465+
const dirpath = pathlib.join(testMocksDir, 'dir');
433466

434467
// eslint-disable-next-line @typescript-eslint/require-await
435468
mockedFsGlob.mockImplementationOnce(async function* () {
@@ -440,15 +473,15 @@ describe('Test getAllTestConfigurations', () => {
440473
yield {
441474
isDirectory: () => true,
442475
name: `project${i}`,
443-
parentPath: '/dir'
476+
parentPath: dirpath,
444477
} as Dirent;
445478
}
446479
}
447480

448481
yield {
449482
isDirectory: () => true,
450483
name: 'node_modules',
451-
parentPath: '/dir'
484+
parentPath: dirpath
452485
} as Dirent;
453486
});
454487

@@ -463,7 +496,7 @@ describe('Test getAllTestConfigurations', () => {
463496
...configs.baseVitestConfig,
464497
test: {
465498
...configs.baseVitestConfig.test!,
466-
root: '/dir/project0',
499+
root: pathlib.join(dirpath, 'project0'),
467500
name: {
468501
label: 'project0',
469502
color: 'black'
@@ -477,7 +510,7 @@ describe('Test getAllTestConfigurations', () => {
477510
...configs.baseVitestConfig,
478511
test: {
479512
...configs.baseVitestConfig.test!,
480-
root: '/dir/project1',
513+
root: pathlib.join(dirpath, 'project1'),
481514
name: {
482515
label: 'project1',
483516
color: 'red'
@@ -491,7 +524,7 @@ describe('Test getAllTestConfigurations', () => {
491524
...configs.baseVitestConfig,
492525
test: {
493526
...configs.baseVitestConfig.test!,
494-
root: '/dir/project2',
527+
root: pathlib.join(dirpath, 'project2'),
495528
name: {
496529
label: 'project2',
497530
color: 'black'

lib/buildtools/src/testing/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ export async function getTestConfiguration(directory: string, watch: boolean): P
9999
const type = typeStr as 'bundle' | 'tab';
100100
return [type, directory];
101101
} catch (error) {
102+
console.log('No package.json found in', directory);
102103
if (!isNodeError(error) || error.code !== 'ENOENT') throw error;
103104
const parentDir = pathlib.resolve(directory, '..');
104105
return findPackageJson(parentDir);

lib/repotools/src/utils.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,9 @@ export function convertToPosixPath(p: string) {
165165
}
166166

167167
/**
168-
* Returns `true` if both paths refer to the same location. This
169-
* function should be OS agnostic
168+
* Returns `true` if both paths refer to the same location.
170169
*/
171170
export function isSamePath(lhs: string, rhs: string) {
172171
const relpath = pathlib.relative(lhs, rhs);
173-
console.log('relpath for', lhs, 'and', rhs, 'is', relpath);
174172
return relpath === '';
175173
}

0 commit comments

Comments
 (0)