Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: migrate message v8 tests from Python to JS #50421

Merged
merged 2 commits into from
Oct 29, 2023
Merged

test: migrate message v8 tests from Python to JS #50421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16f29b3
test: migrate message v8 tests from Python to JS
jahjahLemonade Oct 27, 2023
dd8a959
Cleaned up parallel/test-node-output-v8-warning.mjs
jahjahLemonade Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/message/v8_warning.js → test/fixtures/v8/v8_warning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

require('../common');
require('../../common');

function AsmModule() {
'use asm';
Expand Down
0 test/message/v8_warning.out → test/fixtures/v8/v8_warning.snapshot
View file
Open in desktop
File renamed without changes.
30 changes: 30 additions & 0 deletions test/parallel/test-node-output-v8-warning.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import * as snapshot from '../common/assertSnapshot.js';
import { describe, it } from 'node:test';

function replaceNodeVersion(str) {
return str.replaceAll(process.version, '*');
}

describe('v8 output', { concurrency: true }, () => {
function normalize(str) {
return str.replaceAll(snapshot.replaceWindowsPaths(process.cwd()), '')
.replaceAll(/:\d+/g, ':*')
.replaceAll('/', '*')
.replaceAll('*test*', '*')
.replaceAll('*fixtures*v8*', '*')
.replaceAll('node --', '* --');
Copy link
Member

@codebytere codebytere Jan 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@MoLow @jahjahLemonade the idea here was to make this generic because Electron (and perhaps some other embedders) run these tests with a different executable name (in our case, of course, Electron). As this stands, it fails for the same reason the * was initially added, unintentionally defeating its purpose. We're hitting this now as we upgrade Node.js versions to 20.11.0, which is why this comment is a little delayed 😅

I'd like to try to find a solution for some of these snapshots that's ideally a bit less brittle, given the previous python approach seemed much more flexible and easier to make more or less specific in its comparisons.

}
const common = snapshot
.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceWindowsPaths, replaceNodeVersion);
const defaultTransform = snapshot.transform(common, normalize);
const tests = [
{ name: 'v8/v8_warning.js' },
];
for (const { name, transform } of tests) {
jahjahLemonade marked this conversation as resolved.
Show resolved Hide resolved
it(name, async () => {
await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform);
});
}
});
Loading