Regression test coverage report fials when run locally

[mcking65]

When running the regression test report locally, I get an error that appears to say that ava is not generating any output for standard output:

$ npm run regression-report

> [email protected] regression-report C:\Users\mck\git\APG\main
> node test/util/report

C:\Users\mck\git\APG\main\test\util\report.js:161
  const avaResults = output.stdout.toString();
                                   ^

TypeError: Cannot read property 'toString' of null

[howard-e]

I believe this is no longer happening. Can we close @mcking65?

[nschonni]

@howard-e I believe it is still an issue, but is a Windows-only one

[nschonni]

I ran into the message before when doing work locally and had to temporarily remove the .toString(), but that isn't a correct fix, as it is needed for unix.
Got this one running again today

$ npm run regression-report

> [email protected] regression-report
> node test/util/report

Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:628
            var c = this.buffer.charCodeAt(this._index);
                                ^

TypeError: this.buffer.charCodeAt is not a function
    at Tokenizer.parse (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:628:33)
    at Tokenizer.write (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:111:14)
    at Tokenizer.end (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:117:18)
    at Parser.end (Z:\aria-practices\node_modules\htmlparser2\lib\Parser.js:390:24)
    at parseDocument (Z:\aria-practices\node_modules\htmlparser2\lib\index.js:43:43)
    at Object.parseDOM (Z:\aria-practices\node_modules\htmlparser2\lib\index.js:58:12)
    at processDocumentationInExampleFiles (Z:\aria-practices\test\util\report.js:112:29)
    at Object.<anonymous> (Z:\aria-practices\test\util\report.js:214:1)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)

[howard-e]

I ran into the message before when doing work locally and had to temporarily remove the .toString(), but that isn't a correct fix, as it is needed for unix. Got this one running again today

$ npm run regression-report

> [email protected] regression-report
> node test/util/report

Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:628
            var c = this.buffer.charCodeAt(this._index);
                                ^

TypeError: this.buffer.charCodeAt is not a function
    at Tokenizer.parse (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:628:33)
    at Tokenizer.write (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:111:14)
    at Tokenizer.end (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:117:18)
    at Parser.end (Z:\aria-practices\node_modules\htmlparser2\lib\Parser.js:390:24)
    at parseDocument (Z:\aria-practices\node_modules\htmlparser2\lib\index.js:43:43)
    at Object.parseDOM (Z:\aria-practices\node_modules\htmlparser2\lib\index.js:58:12)
    at processDocumentationInExampleFiles (Z:\aria-practices\test\util\report.js:112:29)
    at Object.<anonymous> (Z:\aria-practices\test\util\report.js:214:1)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)

Okay I see, just ran on Windows as well and can verify I'm seeing the same

[nschonni]

I don't know if it's worth adding separate CI job just to do some sanity checking on Windows or not

[howard-e]

... Got this one running again today

$ npm run regression-report

> [email protected] regression-report
> node test/util/report

Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:628
            var c = this.buffer.charCodeAt(this._index);
                                ^

TypeError: this.buffer.charCodeAt is not a function
    at Tokenizer.parse (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:628:33)
    at Tokenizer.write (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:111:14)
    at Tokenizer.end (Z:\aria-practices\node_modules\htmlparser2\lib\Tokenizer.js:117:18)
    at Parser.end (Z:\aria-practices\node_modules\htmlparser2\lib\Parser.js:390:24)
    at parseDocument (Z:\aria-practices\node_modules\htmlparser2\lib\index.js:43:43)
    at Object.parseDOM (Z:\aria-practices\node_modules\htmlparser2\lib\index.js:58:12)
    at processDocumentationInExampleFiles (Z:\aria-practices\test\util\report.js:112:29)
    at Object.<anonymous> (Z:\aria-practices\test\util\report.js:214:1)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)

@nschonni #2151 should address this and ...

When running the regression test report locally, I get an error that appears to say that ava is not generating any output for standard output:

$ npm run regression-report

> [email protected] regression-report C:\Users\mck\git\APG\main
> node test/util/report

C:\Users\mck\git\APG\main\test\util\report.js:161
  const avaResults = output.stdout.toString();
                                   ^

TypeError: Cannot read property 'toString' of null

... can confirm the original issue still exists

[howard-e]

I don't know if it's worth adding separate CI job just to do some sanity checking on Windows or not

I do think it would be worthwhile to have that

[howard-e]

I ran into the message before when doing work locally and had to temporarily remove the .toString()

What do you think of something like this as a potential Windows specific fix @nschonni?

aria-practices/test/util/report.js

Lines 178 to 186 in 54cd3ff

	const cmd = path.resolve(
	__dirname,
	'..',
	'..',
	'node_modules',
	'ava',
	'cli.js'
	);
	const cmdArgs = [...allTestFiles, '--tap', '-c', '1'];

// L:178
+  const isWin = process.platform === 'win32';
+
+  // fix for running regression-report on windows
+  const cmd = isWin ? 'node.exe' : 'node';
-  const cmd = path.resolve(
+  const avaCmdPath = path.resolve(
     __dirname,
     '..',
     '..',
     'node_modules',
     'ava',
     'cli.js'
   );
-  const cmdArgs = [...allTestFiles, '--tap', '-c', '1'];
+  const cmdArgs = [avaCmdPath, ...allTestFiles, '--tap', '-c', '1'];

   const output = spawnSync(cmd, cmdArgs);
   // ...

[nschonni]

That could work, but what about leveraging the stubs that NPM creates in node_modules/.bin? Since it creates both a ava.cmd and ava, it may work without any special casing for the platform 😄
It's been awhile since I tried figuring out these cross platform issues

[nschonni]

I'm thinking I might have used https://www.npmjs.com/package/cross-spawn in the past to get around some of this stuff

[howard-e]

That could work, but what about leveraging the stubs that NPM creates in node_modules/.bin? Since it creates both a ava.cmd and ava, it may work without any special casing for the platform 😄

Definitely with you there. Had initially tried with npx since it calls into that directory but it seems even then, I have to do npx.cmd ava over just npx ava specifically for Windows from report.js 😅. But it may be worth accessing that folder ourselves

I'm thinking I might have used https://www.npmjs.com/package/cross-spawn in the past to get around some of this stuff

Right, that would work well. I had wanted to avoid adding an extra package for this very small case unless there was no other choice

[howard-e]

I'm thinking I might have used https://www.npmjs.com/package/cross-spawn in the past to get around some of this stuff

I created a branch with your suggestion @nschonni. How does this look to you? https://github.com/w3c/aria-practices/compare/bocoup/fix-issue-959