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.

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

Externalize tests #1881

Draft
wants to merge 9 commits into
base: gh-pages
Choose a base branch
from
Draft

Conversation

p-bakker
Copy link

@p-bakker p-bakker commented Oct 9, 2023

This PR splits the tests and their results into 2 separate files, with the results being stored in JSON format, allowing automated modification, as to open up the possibility to eliminate the manual updating of test results

Additionally, this split makes it possible to create custom Compat Tables, for example with just all the different versions of one environment, including for example the nightly version. Note that are more enhancements possible for custom Compat Tables, even going as far as splitting this repo in a compat-table library and a Compat Table website (github pages) repo, but this are next steps imho

Is does mean that data-common.json is obsolete (and thus removed) as all the test results are not stored in JSON

Closes #1879
Closes #1493
Checks off on of the actions in #965
Building block for #614
Building block for #1790

Did the conversion of data-.js files > tests..js & results-*.json using the following script:

const fs = require('fs');

const extractResults = (entry, result) => {
	if (Array.isArray(entry)) {
		const mapping = {};

		entry.forEach(entry => {
			mapping[entry.name] = extractResults(entry);
		});

		return mapping;
	}

	return entry.res || extractResults(entry.subtests);
}

[
	'es5',
	'es6',
	'es2016plus',
	'esnext',
	'esintl',
	'non-standard',
].forEach(fileName => {
	console.log(`Processing 'data-${fileName}'`);

	const data = require(`./data-${fileName}`);

	// Extract just the results from the data-*.js modules, convert their format and export to result-*.json
	fs.writeFileSync(
		`results-${fileName}.json`, 
		JSON.stringify(
			extractResults(JSON.parse(JSON.stringify(data.tests))),
			null,
			'\t'
		),
		'utf8');

	// Read data-*.js, remove test results and write to tests-*.js
	fs.writeFileSync(
		`./test-${fileName}.js`,
		fs
			.readFileSync(`./data-${fileName}.js`, 'utf8')
			.replace(/,$(\s*)res *: \{.*?$\1}/gms, ''),
		'utf8'
	);

	// Check the conversion
	const tests = require(`./test-${fileName}`).tests;
	const results = require(`./results-${fileName}`);

	if (tests.length !== Object.keys(results).length) console.error(`Test mismatch: tests.length !== Object.keys(results).length (${tests.length} vs. ${Object.keys(results).length})`);

	if (!tests.every((entry, idx) => {
		return !entry.subtests || entry.subtests.length === Object.keys(results[entry.name]).length
	})) console.error(`Subtest mismatch`);
});

- so it becomes possible to automate the updating of the test results
- so it becomes possible to create custom Compat Tables for specialized
environments
@p-bakker p-bakker changed the title Externalize tests DRAFT Externalize tests Oct 9, 2023
@p-bakker p-bakker marked this pull request as draft October 9, 2023 13:05
@p-bakker p-bakker changed the title DRAFT Externalize tests Externalize tests Oct 9, 2023
@p-bakker
Copy link
Author

p-bakker commented Oct 9, 2023

Would it be ok to update the min nodejs version to 14 or 18? Want to use parseArgs either natively (Node 18) or via @pkgjs/parseArgs (Node 14)

@p-bakker p-bakker marked this pull request as ready for review October 9, 2023 16:10
@p-bakker p-bakker force-pushed the externalize-tests branch 2 times, most recently from f37974c to d9b3de6 Compare October 10, 2023 09:05
Add update flag to runners for automatic updates to the result-*.json
files based on the results of the runner
properties

like constructor, __defineGetter__, __defineSetter__ , __lookupGetter
and __lookupSetter__
Some res: {...} remained in the test-*.js files due to slightly
different formatting used and therefor missed by the regex replace that
was part of the conversion.

As all the test results are extracted into .json files, data-commom is
no longer used, thus removed
@p-bakker p-bakker marked this pull request as draft October 13, 2023 19:14
@p-bakker
Copy link
Author

(re)marked this as Draft again, as I'm contemplating some additional changes, but would already appreciate some feedback on the general direction of extracting the results into their own json files.

Has that any change of getting accepted?

@p-bakker p-bakker mentioned this pull request Oct 13, 2023
@p-bakker
Copy link
Author

Any feedback anyone?

Don't want to spend more time on this if the general direction of this PR, namely splitting the tests and results into separate files, is gonna be a no-go to begin with

@p-bakker
Copy link
Author

I guess the change in where the test results are stored would break the mechanism in Babel that downloads and converts the compat data, but imho it would be easy to adapt Babel accordingly

p-bakker added a commit to rhino/rhino.github.io that referenced this pull request May 21, 2024
Until such time when we can host proper custom compat tables based on the latest iteration of compat-table, see compat-table/compat-table#1881
@p-bakker
Copy link
Author

p-bakker commented Jun 1, 2024

@ljharb @chicoxyzzy any chance to get some feedback on the general idea to extract the results out from the data-*.js files and into .json files?

Is it worth my time to finish this PR and get it out of draft status so it can be considered for merge? Or is this general idea dead in the water?

@ljharb
Copy link
Member

ljharb commented Jun 2, 2024

It's a pretty major change so it's hard to develop an opinion about it, I think :-)

@p-bakker
Copy link
Author

p-bakker commented Jun 2, 2024

Ok, but what does that mean exactly? 🙂

Does it mean you and/or others are actively thinking about whether or not this change is something you're willing to accept?

Or do you need something additional from me to be able to make that decision?

Or ....?

@ljharb
Copy link
Member

ljharb commented Jun 2, 2024

It means I'm actively thinking about it. I can't speak for the other maintainers.

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.

Externalize results from tests Store Test Results In JSON
2 participants