Wrap @web/test-runner binary #65

bearfriend · 2023-07-11T18:33:46Z

US154135: Visual-diff: wrap web-test-runner binary

Use is largely the same. Notable changes:

Commands: wtr/web-test-runner -> d2l-test
Browser names: chromium -> chrome, webkit -> safari
Config file:
- web-test-runner.config.js -> d2l-test.config.js
- Now just a simple POJO export. No more createConfig or getBrowsers
Options:
- --files is now an explicit option
- --group is now the implicit option, with unit being the default (i.e. d2l-test and d2l-test unit run the unit group)
- --filter no longer adds leading and trailing wildcards. So, in core d2l-test -f list will only find list.test.js. Add your own wildcards to match more: -f list* or -f *list*

Most wtr options will pass through, but some are explicitly disabled to avoid overwriting the d2l-test config.

bin/test.js

bearfriend · 2023-07-11T18:36:35Z

bin/test.js

 		const requestedBrowsers = ALLOWED_BROWSERS.filter(b => cliArgs?.[b]);
 		this.#requestedBrowsers = requestedBrowsers.length && requestedBrowsers;
 	}

 	get #defaultConfig() {
 		return {
-			files: this.#getPattern('test'),


Removing this disables the default group

bearfriend · 2023-07-11T18:37:13Z

bin/test.js

@@ -126,44 +138,34 @@ export class WTRConfig {
 	}

 	#getPattern(type) {
-		const pattern = this.#cliArgs.files || this.pattern(type);
+		const pattern = [].concat(this.#cliArgs.files || this.pattern(type));


Allows --files option (which will be an array) to be filtered properly

bearfriend · 2023-07-11T18:39:56Z

bin/test.js

-					return `+(${fileGlobs.join('|')})`;
-				});
-			});
+					return `+(${fileGlobs.join('|') || fileGlob})`;


Handles bad POSIX CLI globs gracefully.

--files ./*.test.js is converted by the OS to a list of all matching files (with no * characters) before being sent to the process.
--files './*.test.js' is what should be used. We can add standard POSIX glob note to docs.

bearfriend · 2023-07-11T18:41:13Z

bin/test.js

-		if (!group || group === 'default') {
-			if (playwright) {
-				console.warn('Warning: reducedMotion disabled. Use the unit group to enable reducedMotion.');
-			} else {
-				console.warn('Warning: Running with puppeteer, reducedMotion disabled. Use the unit group to use playwright with reducedMotion enabled');
-			}
-		}


No longer necessary since we are disabling default group and defaulting to unit group (and forcing all browsers options to playwright browsers).

bearfriend · 2023-07-11T18:41:41Z

bin/test.js

 		});

-		if (vdiff) {
+		if (group === 'vdiff') {


No more vdiff: true. Just use the group to enable it.

src/server/index.js

src/server/visual-diff-plugin.js

bearfriend · 2023-07-11T18:44:07Z

src/server/visual-diff-reporter.js

@@ -112,7 +112,7 @@ export function visualDiffReporter({ reportResults = true } = {}) {
 			cpSync(inputDir, tempDir, { force: true, recursive: true });
 			writeFileSync(join(tempDir, 'data.js'), `export default ${json};`);

-			execSync(`rollup -c ${join(__dirname, './rollup.config.js')}`);
+			execSync(`npx rollup -c ${join(__dirname, './rollup.config.js')}`, { stdio: 'pipe' });


npx fixes command not found error, which I believe is necessary now the way this runs.

stdio fixes the double output, though I'm not sure why, since it's supposed to be the default. May need to make this ignore if it spits out other bad things.

bearfriend · 2023-07-12T12:53:38Z

Tests in progress

bin/test.js

src/server/visual-diff-reporter.js

bin/test.js

test/browser/d2l-test.config.js

bin/test.js

package.json

dlockhart · 2023-07-20T16:02:55Z

test/server/cli/test.test.js

+
+	it('should not forward disallowed or stolen options', async() => {
+		const disallowed = ['--browsers', '--playwright', '--puppeteer', '--groups'];
+		const stolen = ['--config'];


😆 stolen

Hah. I tried to avoid it at first but then when I got to borrowed it seemed like the most accurate word.

src/server/cli/test.js

svanherk · 2023-07-20T16:03:01Z

src/server/cli/test.js

+			order: 1
+		},
+		{
+			name: 'manual',


Not sure if I raised this here or the other PR, this will still be removed in a future PR?

svanherk · 2023-07-20T16:04:30Z

src/server/cli/test.js

+
+		// d2l-test options
+		{
+			name: 'chrome',


One thing I was thinking about - does this affect the browser names the runner has? Will the goldens be stored in chrome and safari folders, or still chromium and webkit?

Nothing else changes. I did start second-guessing this because literally everything else will not match these. Directories, test output, report tabs, (playwright/wtr options).

Yeah, it's convenient but makes me like it a bit less if everything else will still say the old name

I'd be happy to switch what the report shows if that's what we're worried about. The output in the console though will still say "chromium" and "webkit". Surprised Firefox doesn't resort to "gecko".

I think the weirdest part for me would be the folders not matching, which I guess we could change by having a map in the plugin.

src/server/wtr-config.js

test/browser/d2l-test.config.js

package.json

ghost · 2023-07-21T13:40:01Z

🎉 This PR is included in version 0.18.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

bearfriend commented Jul 11, 2023

View reviewed changes

bin/test.js Outdated Show resolved Hide resolved

bearfriend commented Jul 11, 2023

View reviewed changes

src/server/index.js Outdated Show resolved Hide resolved

bearfriend commented Jul 11, 2023

View reviewed changes

src/server/visual-diff-plugin.js Outdated Show resolved Hide resolved

bearfriend commented Jul 11, 2023

View reviewed changes

bearfriend marked this pull request as ready for review July 12, 2023 12:53

bearfriend requested a review from a team as a code owner July 12, 2023 12:53

dlockhart reviewed Jul 12, 2023

View reviewed changes

bin/test.js Outdated Show resolved Hide resolved

bin/test.js Outdated Show resolved Hide resolved

src/server/visual-diff-reporter.js Show resolved Hide resolved

svanherk reviewed Jul 12, 2023

View reviewed changes

bin/test.js Outdated Show resolved Hide resolved

bin/test.js Outdated Show resolved Hide resolved

test/browser/d2l-test.config.js Outdated Show resolved Hide resolved

bearfriend added 16 commits July 19, 2023 08:20

Add d2l-test binary

030f8a3

Map browser names in getBrowsers

fde5e92

Cleanup

0b8bb58

Fix option handling

ba6172a

Move WTRConfig class back out of test binary

50274ed

Add option aliases

f9a1ab0

Fix imports

639cd8a

Delete server/index.js

f69e831

Remove HEY!

754e0fc

Install config-loader

f098c03

Remove options that wtr shouldn't use

31fe6f8

Add --help

719c7b5

Add unit group when used

4e18fb1

Update filter comment

71332f5

Add --help details

65a67c7

Use proper node imports

b4ff126

bearfriend added 6 commits July 19, 2023 08:22

Add support for filtering custom group files

e4719e9

unit -> test

14d85d2

Check for unknown group

1858d00

Reorganize and fix/add tests

0236ee4

Clarify --golden description

92609c2

Simplify pattern

b2d4060

bearfriend force-pushed the dgleckler/bin-wrap branch from 0829a96 to b2d4060 Compare July 19, 2023 12:22

bearfriend commented Jul 19, 2023

View reviewed changes

test/browser/d2l-test.config.js Outdated Show resolved Hide resolved

Fix lint

904434c

bearfriend mentioned this pull request Jul 19, 2023

Add fixture pausing and --open option to replace --manual #71

Closed

Fix test

0a68bd0

bearfriend requested review from dlockhart and svanherk July 20, 2023 14:58

dlockhart reviewed Jul 20, 2023

View reviewed changes

svanherk reviewed Jul 20, 2023

View reviewed changes

test/browser/d2l-test.config.js Outdated Show resolved Hide resolved

bearfriend added 2 commits July 20, 2023 14:11

Fix option descriptions

7a87801

Rename things

a131243

bearfriend force-pushed the dgleckler/bin-wrap branch from 4fa0c84 to a131243 Compare July 20, 2023 18:24

svanherk reviewed Jul 20, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

Fix test:vdiff script

b90739b

dlockhart approved these changes Jul 20, 2023

View reviewed changes

bearfriend added 3 commits July 20, 2023 15:39

Accept all browser names

674aab2

Fix browser tests

b972f3d

Dedupe browsers

c2784fb

svanherk approved these changes Jul 21, 2023

View reviewed changes

bearfriend merged commit 385dc31 into main Jul 21, 2023

bearfriend deleted the dgleckler/bin-wrap branch July 21, 2023 13:38

ghost added the released label Jul 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap @web/test-runner binary #65

Wrap @web/test-runner binary #65

bearfriend commented Jul 11, 2023 •

edited

Loading

bearfriend Jul 11, 2023 •

edited

Loading

bearfriend Jul 11, 2023

bearfriend Jul 11, 2023

bearfriend Jul 11, 2023 •

edited

Loading

bearfriend Jul 11, 2023

bearfriend Jul 11, 2023

bearfriend commented Jul 12, 2023

dlockhart Jul 20, 2023

bearfriend Jul 20, 2023

svanherk Jul 20, 2023

svanherk Jul 20, 2023

bearfriend Jul 20, 2023

svanherk Jul 20, 2023

dlockhart Jul 20, 2023

svanherk Jul 20, 2023

ghost commented Jul 21, 2023

Wrap @web/test-runner binary #65

Wrap @web/test-runner binary #65

Conversation

bearfriend commented Jul 11, 2023 • edited Loading

bearfriend Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bearfriend Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bearfriend commented Jul 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 21, 2023

bearfriend commented Jul 11, 2023 •

edited

Loading

bearfriend Jul 11, 2023 •

edited

Loading

bearfriend Jul 11, 2023 •

edited

Loading