Skip to content

Commit

Permalink
Clean up the test bootstrapping
Browse files Browse the repository at this point in the history
The tests were failing because (apparently) vite wasn't analyzing the
`index.html` file properly. This resulted in multiple copies of the
validator code getting included in the tests (via chunks and via the
`.ts` module itself).

To make matters worse, the validator code included in the chunk was
missing the code that detects duplicates, so the only failure symptom
was an error reporting that the `COMPUTE` symbol wasn't present on an
instance of `Tag`.

Being explicit about entry points fixed the problem and made the tests
pass. I assume that something about the previous `index.html` file
wasn't being analyzed properly, so I cleaned up the code to make the
glob dependency extremely explicit and moved all of the rest of the code
into the `setup-harness` module.

Tests pass with the new setup, so I think we're good to go.
  • Loading branch information
wycats committed Jan 21, 2025
1 parent e442e1f commit da90f20
Show file tree
Hide file tree
Showing 11 changed files with 398 additions and 73 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,6 @@ jobs:
- uses: wyvox/action@v1
with:
pnpm-args: '--no-lockfile'
node-version: 20.1.0
node-version: 22.13.0
repo-token: ${{ secrets.GITHUB_TOKEN }}
- run: pnpm test
1 change: 1 addition & 0 deletions benchmark/benchmarks/krausest/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
packages/
pnpm-lock.yaml
13 changes: 11 additions & 2 deletions bin/run-tests.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,15 @@ const browser = await puppeteer.launch({
console.log('[ci] puppeteer launched');

try {
console.log('[ci] navigating to new page');
const page = await browser.newPage();
console.log('[ci] done navigating');

await /** @type {Promise<void>} */ (
console.log('[ci] waiting for console');
const promise = /** @type {Promise<void>} */ (
new Promise((fulfill, reject) => {
page.on('console', (msg) => {
console.error(msg.text());
const location = msg.location();
const text = msg.text();

Expand All @@ -77,8 +81,13 @@ try {
});
})
);
console.log('[ci] done waiting');

await page.goto('http://localhost:60173?hidepassed&ci');
console.log('[ci] navigating to test page');
void page.goto('http://localhost:60173?hidepassed&ci');
console.log('[ci] done navigating');

await promise;
} catch {
await browser.close();
process.exit(1);
Expand Down
33 changes: 3 additions & 30 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,10 @@

<body>
<script type="module">
// Do this first so that `@glimmer-workspace/integration-tests` will see the global
// `QUnit`. We should change the code structure here so that we don't
// rely on the global to bootstrap QUnit, but this is fine for now.
const QUnit = await import("qunit");
QUnit.config.autostart = false;
</script>
<script type="module">
import { setupQunit } from "@glimmer-workspace/integration-tests";
const { smokeTest } = await setupQunit();

const packages = await import.meta.glob("./packages/@glimmer/*/test/**/*-test.ts");
const integrationTestFiles = await import.meta.glob(
"./packages/@glimmer-workspace/*/test/**/*-test.ts",
import { runTests } from "@glimmer-workspace/integration-tests";
await runTests(
import.meta.glob("./packages/{@glimmer,@glimmer-workspace}/*/test/**/*-test.ts"),
);

let smokeTestFile = "./packages/@glimmer-workspace/integration-tests/test/smoke-test.ts";

// evaluate the tests before starting QUnit
for (const [name, pkg] of Object.entries(packages)) {
await pkg();
}

for (const [name, pkg] of Object.entries(integrationTestFiles)) {
if (name === smokeTestFile && !smokeTest) {
continue;
}

await pkg();
}

QUnit.start();
</script>
</body>
</html>
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"turbo": "^2.3.3",
"typescript": "^5.7.3",
"typescript-eslint": "^8.19.0",
"vite": "^5.4.11",
"vite": "^6.0.10",
"zx": "^8.3.0"
},
"changelog": {
Expand Down
25 changes: 25 additions & 0 deletions packages/@glimmer-workspace/integration-tests/lib/setup-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,31 @@ import { debug } from '@glimmer/validator';
import { autoRegister } from 'js-reporters';
import { default as QUnit } from 'qunit';

const SMOKE_TEST_FILE = './packages/@glimmer-workspace/integration-tests/test/smoke-test.ts';

export async function runTests(packages: Record<string, () => Promise<void>>) {
const { smokeTest } = await setupQunit();
return bootQunit(packages, { smokeTest });
}

export async function bootQunit(
packages: Record<string, () => Promise<void>>,
options: { smokeTest?: boolean } = {}
) {
const { smokeTest } = options;

for (const [name, pkg] of Object.entries(packages)) {
if (name === SMOKE_TEST_FILE && !smokeTest) {
console.log('skipping', name);
continue;
}

await pkg();
}

QUnit.start();
}

export async function setupQunit() {
const qunitLib: QUnit = await import('qunit');
await import('qunit/qunit/qunit.css');
Expand Down
3 changes: 3 additions & 0 deletions packages/@glimmer-workspace/integration-tests/lib/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ import type { IteratorDelegate } from '@glimmer/reference';
import type { TestBase } from 'qunit';
import setGlobalContext from '@glimmer/global-context';
import { consumeTag, dirtyTagFor, tagFor } from '@glimmer/validator';
import QUnit from 'qunit';

import { scheduleDidDestroy, scheduleWillDestroy } from './base-env';
import { NativeIteratorDelegate } from './modes/env';

QUnit.config.autostart = false;

let actualDeprecations: string[] = [];

// Override the types on Assert to add our own helper
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { jitSuite, RenderTest, test } from "@glimmer-workspace/integration-tests";
import { jitSuite, RenderTest, test } from '@glimmer-workspace/integration-tests';

class SmokeTests extends RenderTest {
static suiteName = 'Smoke Tests';
Expand Down
4 changes: 3 additions & 1 deletion packages/@glimmer/validator/lib/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ const CONSTANT_TAG_ID: ICONSTANT_TAG_ID = 3;

//////////

console.log('Defining COMPUTE');

Check failure on line 44 in packages/@glimmer/validator/lib/validators.ts

View workflow job for this annotation

GitHub Actions / Linting

Unexpected console statement
export const COMPUTE: TagComputeSymbol = Symbol('TAG_COMPUTE') as TagComputeSymbol;
Reflect.set(globalThis, 'COMPUTE_SYMBOL', COMPUTE);

//////////

Expand Down Expand Up @@ -113,7 +115,7 @@ class MonomorphicTagImpl<T extends MonomorphicTagId = MonomorphicTagId> {
public subtag: Tag | Tag[] | null = null;
private subtagBufferCache: Revision | null = null;

[TYPE]: T;
declare [TYPE]: T;

constructor(type: T) {
this[TYPE] = type;
Expand Down
Loading

0 comments on commit da90f20

Please sign in to comment.