Skip to content

Commit 3a20b5b

Browse files
chore(cli): speedup internal tests (#698)
* step 1 - no browser & preview if not test on page * test with devtools-json * step by step :) * add dummy flavor * more to test pnpm * kitOnly * installCmd * Revert changes back to fe61de7 state This reverts all changes made after commit fe61de7 (step 1 - no browser & preview if not test on page) Reverted commits: - cfe2965: test with devtools-json - 1db5b78: step by step :) - 167fdbd: add dummy flavor - b376118: more to test pnpm - 41cb8ff: kitOnly - a6b528c: installCmd * install & build only ts (when no extra things after) * speed up by using async exec * rework playwright test * rename `skipBrowser` and don't run `prepareServer` when `browser` is disabled * these tests need to have the browser and server running * tweak storybook test * fix drizzle test * use locators * oops * ok let these be sync * maybe this * disable file parallelism * test also limiting concurrency * ugh * remove concurrency only for tests that run shell commands * revert before i lose my sanity * pipe io * skip storybook for now * now lets do async shell cmd only on build and install * yea i though as much - reverting * dont skip storybook --------- Co-authored-by: AdrianGonz97 <31664583+AdrianGonz97@users.noreply.github.com>
1 parent bf36ebc commit 3a20b5b

File tree

11 files changed

+79
-91
lines changed

11 files changed

+79
-91
lines changed

.changeset/dull-islands-lead.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'sv': patch
3+
---
4+
5+
chore(cli): speedup internal tests

packages/addons/_tests/_setup/suite.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import fs from 'node:fs';
22
import path from 'node:path';
3-
import { execSync } from 'node:child_process';
3+
import { promisify } from 'node:util';
4+
import { exec, execSync } from 'node:child_process';
45
import * as vitest from 'vitest';
56
import { installAddon, type AddonMap, type OptionMap } from 'sv';
67
import {
@@ -10,29 +11,38 @@ import {
1011
type CreateProject,
1112
type ProjectVariant
1213
} from 'sv/testing';
13-
import { chromium, type Browser, type Page } from '@playwright/test';
14+
import { chromium, type Browser, type BrowserContext, type Page } from '@playwright/test';
1415

1516
const cwd = vitest.inject('testDir');
1617
const templatesDir = vitest.inject('templatesDir');
1718
const variants = vitest.inject('variants');
1819

20+
export const execAsync = promisify(exec);
21+
1922
type Fixtures<Addons extends AddonMap> = {
2023
page: Page;
2124
run(variant: ProjectVariant, options: OptionMap<Addons>): Promise<string>;
2225
};
2326

24-
export function setupTest<Addons extends AddonMap>(addons: Addons) {
27+
export function setupTest<Addons extends AddonMap>(
28+
addons: Addons,
29+
options?: { browser?: boolean }
30+
) {
2531
const test = vitest.test.extend<Fixtures<Addons>>({} as any);
2632

33+
const withBrowser = options?.browser ?? true;
34+
2735
let create: CreateProject;
2836
let browser: Browser;
2937

30-
vitest.beforeAll(async () => {
31-
browser = await chromium.launch();
32-
return async () => {
33-
await browser.close();
34-
};
35-
});
38+
if (withBrowser) {
39+
vitest.beforeAll(async () => {
40+
browser = await chromium.launch();
41+
return async () => {
42+
await browser.close();
43+
};
44+
});
45+
}
3646

3747
vitest.beforeAll(({ name }) => {
3848
const testName = path.dirname(name).split('/').at(-1)!;
@@ -59,8 +69,11 @@ export function setupTest<Addons extends AddonMap>(addons: Addons) {
5969

6070
// runs before each test case
6171
vitest.beforeEach<Fixtures<Addons>>(async (ctx) => {
62-
const browserCtx = await browser.newContext();
63-
ctx.page = await browserCtx.newPage();
72+
let browserCtx: BrowserContext;
73+
if (withBrowser) {
74+
browserCtx = await browser.newContext();
75+
ctx.page = await browserCtx.newPage();
76+
}
6477
ctx.run = async (variant, options) => {
6578
const cwd = create({ testId: ctx.task.id, variant });
6679

@@ -81,7 +94,9 @@ export function setupTest<Addons extends AddonMap>(addons: Addons) {
8194
};
8295

8396
return async () => {
84-
await browserCtx.close();
97+
if (withBrowser) {
98+
await browserCtx.close();
99+
}
85100
// ...other tear downs
86101
};
87102
});

packages/addons/_tests/devtools-json/test.ts

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,11 @@ import devtoolsJson from '../../devtools-json/index.ts';
44
import fs from 'node:fs';
55
import path from 'node:path';
66

7-
const { test, variants, prepareServer } = setupTest({ devtoolsJson } as {
8-
devtoolsJson?: typeof devtoolsJson;
9-
});
7+
const { test, variants } = setupTest({ devtoolsJson }, { browser: false });
108

11-
test.concurrent.for(variants)('default - %s', async (variant, { page, ...ctx }) => {
9+
test.concurrent.for(variants)('default - %s', async (variant, ctx) => {
1210
const cwd = await ctx.run(variant, { devtoolsJson: {} });
1311

14-
const { close } = await prepareServer({ cwd, page });
15-
// kill server process when we're done
16-
ctx.onTestFinished(async () => await close());
17-
1812
const ext = variant.includes('ts') ? 'ts' : 'js';
1913
const viteFile = path.resolve(cwd, `vite.config.${ext}`);
2014
const viteContent = fs.readFileSync(viteFile, 'utf8');
@@ -26,25 +20,3 @@ test.concurrent.for(variants)('default - %s', async (variant, { page, ...ctx })
2620
// Check if it's called
2721
expect(viteContent).toContain(`devtoolsJson()`);
2822
});
29-
30-
test.concurrent.for(variants)(
31-
'without selecting the addon specifically - %s',
32-
async (variant, { page, ...ctx }) => {
33-
const cwd = await ctx.run(variant, {});
34-
35-
const { close } = await prepareServer({ cwd, page });
36-
// kill server process when we're done
37-
ctx.onTestFinished(async () => await close());
38-
39-
const ext = variant.includes('ts') ? 'ts' : 'js';
40-
const viteFile = path.resolve(cwd, `vite.config.${ext}`);
41-
const viteContent = fs.readFileSync(viteFile, 'utf8');
42-
43-
// Check if we have the import part
44-
expect(viteContent).toContain(`import devtoolsJson from`);
45-
expect(viteContent).toContain(`vite-plugin-devtools-json`);
46-
47-
// Check if it's called
48-
expect(viteContent).toContain(`devtoolsJson()`);
49-
}
50-
);

packages/addons/_tests/drizzle/test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import path from 'node:path';
33
import process from 'node:process';
44
import { fileURLToPath } from 'node:url';
55
import { execSync } from 'node:child_process';
6-
import * as vitest from 'vitest';
6+
import { beforeAll } from 'vitest';
77
import { expect } from '@playwright/test';
88
import { setupTest } from '../_setup/suite.ts';
99
import drizzle from '../../drizzle/index.ts';
@@ -14,7 +14,7 @@ const { test, variants, prepareServer } = setupTest({ drizzle });
1414
// only linux is supported for running docker containers in github runners
1515
const noDocker = process.env.CI && process.platform !== 'linux';
1616

17-
vitest.beforeAll(() => {
17+
beforeAll(() => {
1818
if (noDocker) return;
1919
const cwd = path.dirname(fileURLToPath(import.meta.url));
2020
execSync('docker compose up --detach', { cwd, stdio: 'pipe' });
@@ -64,6 +64,6 @@ test.concurrent.for(testCases)(
6464
// kill server process when we're done
6565
ctx.onTestFinished(async () => await close());
6666

67-
expect(await page.$('[data-testid]')).toBeTruthy();
67+
expect(page.locator('[data-testid]')).toBeTruthy();
6868
}
6969
);
Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,22 @@
11
import fs from 'node:fs';
22
import path from 'node:path';
33
import { execSync } from 'node:child_process';
4-
import { expect } from '@playwright/test';
54
import { setupTest } from '../_setup/suite.ts';
65
import eslint from '../../eslint/index.ts';
76

8-
const { test, variants, prepareServer } = setupTest({ eslint });
7+
const { test, variants } = setupTest({ eslint }, { browser: false });
98

10-
test.concurrent.for(variants)('core - %s', async (variant, { page, ...ctx }) => {
9+
test.concurrent.for(variants)('core - %s', async (variant, { expect, ...ctx }) => {
1110
const cwd = await ctx.run(variant, { eslint: {} });
1211

13-
const { close } = await prepareServer({ cwd, page });
14-
// kill server process when we're done
15-
ctx.onTestFinished(async () => await close());
16-
1712
const unlintedFile = 'let foo = "";\nif (Boolean(foo)) {\n//\n}';
1813
fs.writeFileSync(path.resolve(cwd, 'src/lib/foo.js'), unlintedFile, 'utf8');
1914

20-
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).toThrowError();
15+
expect(() => execSync('pnpm install', { cwd, stdio: 'pipe' })).not.toThrow();
16+
17+
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).toThrow();
2118

22-
expect(() => execSync('pnpm eslint --fix .', { cwd, stdio: 'inherit' })).not.toThrowError();
19+
expect(() => execSync('pnpm eslint --fix .', { cwd, stdio: 'pipe' })).not.toThrow();
2320

24-
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).not.toThrowError();
21+
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).not.toThrow();
2522
});

packages/addons/_tests/mdsvex/test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ test.concurrent.for(variants)('core - %s', async (variant, { page, ...ctx }) =>
2020
// kill server process when we're done
2121
ctx.onTestFinished(async () => await close());
2222

23-
expect(await page.$('.mdsvex h1')).toBeTruthy();
24-
expect(await page.$('.mdsvex h2')).toBeTruthy();
25-
expect(await page.$('.mdsvex p')).toBeTruthy();
23+
expect(page.locator('.mdsvex h1')).toBeTruthy();
24+
expect(page.locator('.mdsvex h2')).toBeTruthy();
25+
expect(page.locator('.mdsvex p')).toBeTruthy();
2626
});
2727

2828
function addFixture(cwd: string, variant: string) {
Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
1-
import { expect } from '@playwright/test';
1+
import fs from 'node:fs';
2+
import path from 'node:path';
23
import { setupTest } from '../_setup/suite.ts';
34
import playwright from '../../playwright/index.ts';
45

5-
const { test, variants, prepareServer } = setupTest({ playwright });
6+
const { test, variants } = setupTest({ playwright }, { browser: false });
67

7-
test.concurrent.for(variants)('core - %s', async (variant, { page, ...ctx }) => {
8+
test.concurrent.for(variants)('core - %s', async (variant, { expect, ...ctx }) => {
89
const cwd = await ctx.run(variant, { playwright: {} });
910

10-
const { close } = await prepareServer({ cwd, page });
11-
// kill server process when we're done
12-
ctx.onTestFinished(async () => await close());
11+
const ext = variant.includes('ts') ? 'ts' : 'js';
12+
const playwrightConfig = path.resolve(cwd, `playwright.config.${ext}`);
13+
const configContent = fs.readFileSync(playwrightConfig, 'utf8');
1314

14-
expect(true).toBe(true);
15+
// Check if we have the imports
16+
expect(configContent).toContain(`import { defineConfig } from`);
17+
expect(configContent).toContain(`@playwright/test`);
18+
19+
// Check if it's called
20+
expect(configContent).toContain(`export default defineConfig({`);
1521
});
Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,22 @@
11
import fs from 'node:fs';
22
import path from 'node:path';
33
import { execSync } from 'node:child_process';
4-
import { expect } from '@playwright/test';
54
import { setupTest } from '../_setup/suite.ts';
65
import prettier from '../../prettier/index.ts';
76

8-
const { test, variants, prepareServer } = setupTest({ prettier });
7+
const { test, variants } = setupTest({ prettier }, { browser: false });
98

10-
test.concurrent.for(variants)('core - %s', async (variant, { page, ...ctx }) => {
9+
test.concurrent.for(variants)('core - %s', async (variant, { expect, ...ctx }) => {
1110
const cwd = await ctx.run(variant, { prettier: {} });
1211

13-
const { close } = await prepareServer({ cwd, page });
14-
// kill server process when we're done
15-
ctx.onTestFinished(async () => await close());
16-
1712
const unformattedFile = 'const foo = "bar"';
1813
fs.writeFileSync(path.resolve(cwd, 'src/lib/foo.js'), unformattedFile, 'utf8');
1914

20-
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).toThrowError();
15+
expect(() => execSync('pnpm install', { cwd, stdio: 'pipe' })).not.toThrow();
16+
17+
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).toThrow();
2118

22-
expect(() => execSync('pnpm format', { cwd, stdio: 'pipe' })).not.toThrowError();
19+
expect(() => execSync('pnpm format', { cwd, stdio: 'pipe' })).not.toThrow();
2320

24-
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).not.toThrowError();
21+
expect(() => execSync('pnpm lint', { cwd, stdio: 'pipe' })).not.toThrow();
2522
});

packages/addons/_tests/storybook/test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,18 @@ import eslint from '../../eslint/index.ts';
1010
const { test, variants, prepareServer } = setupTest({ storybook, eslint });
1111

1212
let port = 6006;
13+
const CI = Boolean(process.env.CI);
1314

1415
beforeAll(() => {
15-
if (process.env.CI) {
16+
if (CI) {
1617
// prefetch the storybook cli during ci to reduce fetching errors in tests
1718
execSync('pnpx create-storybook@latest --version');
1819
}
1920
});
2021

2122
test.for(variants)(
2223
'storybook loaded - %s',
23-
{ concurrent: !process.env.CI },
24+
{ concurrent: !CI },
2425
async (variant, { page, ...ctx }) => {
2526
const cwd = await ctx.run(variant, { storybook: {}, eslint: {} });
2627

@@ -33,7 +34,7 @@ test.for(variants)(
3334
// kill server process when we're done
3435
ctx.onTestFinished(async () => await close());
3536

36-
expect(await page.$('main .sb-bar')).toBeTruthy();
37-
expect(await page.$('#storybook-preview-wrapper')).toBeTruthy();
37+
expect(page.locator('main .sb-bar')).toBeTruthy();
38+
expect(page.locator('#storybook-preview-wrapper')).toBeTruthy();
3839
}
3940
);

packages/addons/_tests/sveltekit-adapter/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { expect } from '@playwright/test';
2-
import { readFile } from 'node:fs/promises';
31
import { join } from 'node:path';
2+
import { readFile } from 'node:fs/promises';
3+
import { expect } from '@playwright/test';
44
import sveltekitAdapter from '../../sveltekit-adapter/index.ts';
55
import { setupTest } from '../_setup/suite.ts';
66

0 commit comments

Comments
 (0)