Skip to content

test: fail if file does not exist#2369

Open
Lightning00Blade wants to merge 3 commits into
mainfrom
don't-silsenc-a-failed
Open

test: fail if file does not exist#2369
Lightning00Blade wants to merge 3 commits into
mainfrom
don't-silsenc-a-failed

Conversation

@Lightning00Blade

Copy link
Copy Markdown
Collaborator

Given we run test locally and that this does not change often one can just run the command once in a while to fix the local setup.

Failed tests:

✖ matches snapshot if exists (0.78188ms)
  Error: THIRD_PARTY_NOTICES does not exist, run `npm ci && npm run bundle`
      at TestContext.<anonymous> (file:///usr/local/google/home/nvitkov/chrome-devtools-mcp/build/tests/third_party_notices.test.js:13:19)
      at Test.runInAsyncScope (node:async_hooks:227:14)
      at Test.run (node:internal/test_runner/test:1325:25)
      at Test.start (node:internal/test_runner/test:1191:17)
      at node:internal/test_runner/test:1792:71
      at node:internal/per_context/primordials:466:82
      at new Promise (<anonymous>)
      at new SafePromise (node:internal/per_context/primordials:435:3)
      at node:internal/per_context/primordials:466:9
      at Array.map (<anonymous>)
✖ THIRD_PARTY_NOTICES (1.41613ms)
  '1 subtest failed'
  

@Lightning00Blade
Lightning00Blade requested a review from OrKoN July 15, 2026 14:12

@OrKoN OrKoN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will make the default test run always fail on a clean build (Additionally, npm build might not work after npm run bundle). I suggest we move this test to a separate test step (npm run test-bundle). WDYT?

@Lightning00Blade

Copy link
Copy Markdown
Collaborator Author

Separate test sound good.

@Lightning00Blade
Lightning00Blade force-pushed the don't-silsenc-a-failed branch from b159a71 to bce9831 Compare July 16, 2026 13:43
Comment thread .github/workflows/run-tests.yml Outdated
Comment on lines +78 to +79
${{ matrix.os == 'windows-latest' && '--test-concurrency=1' || '' }}
${{ github.event_name == 'merge_group' && '--retry' || '' }}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might not need it for one test

Comment thread scripts/test.mjs Outdated
files.push('build/tests/**/*.test.js');
const {glob} = await import('node:fs/promises');
for await (const tsFile of glob('tests/**/*.test.ts')) {
if (tsFile.replace(/\\/g, '/') === 'tests/third_party_notices.test.ts') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import {describe, it} from 'node:test';

describe('THIRD_PARTY_NOTICES', () => {
it('matches snapshot if exists', t => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('matches snapshot', t => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants