Amphtml: Unit test loader fails to parse jsx import assertions while computing set of tests to run

[powerivq]

I encountered a problem on Travis when running gulp unit --nobuild --headless --local_changes. Error is:

[20:51:52] 'unit' errored after 10 s
[20:51:52] SyntaxError: Unexpected token, expected ";" (24:52)
    at Object._raise (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:766:17)
    at Object.raiseWithData (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:759:17)
    at Object.raise (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:753:17)
    at Object.unexpected (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:8966:16)
    at Object.semicolon (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:8948:40)
    at Object.parseImport (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:12814:10)
    at Object.parseStatementContent (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:11531:27)
    at Object.parseStatement (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:11431:17)
    at Object.parseBlockOrModuleBlockBody (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:12013:25)
    at Object.parseBlockBody (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:11999:10)
    at Object.parseTopLevel (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:11362:10)
    at Object.parse (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:13045:10)
    at Object.parse (/Users/shihuaz/work/amphtml/node_modules/@babel/parser/lib/index.js:13098:38)
    at Object.parse (/Users/shihuaz/work/amphtml/node_modules/list-imports-exports/index.js:8:20)
    at getImports (/Users/shihuaz/work/amphtml/build-system/tasks/runtime-test/helpers-unit.js:90:40)
    at /Users/shihuaz/work/amphtml/build-system/tasks/runtime-test/helpers-unit.js:119:11
    at Array.forEach (<anonymous>)
    at getJsFilesFor (/Users/shihuaz/work/amphtml/build-system/tasks/runtime-test/helpers-unit.js:117:18)
    at /Users/shihuaz/work/amphtml/build-system/tasks/runtime-test/helpers-unit.js:215:34
    at Array.forEach (<anonymous>)
    at unitTestsToRun (/Users/shihuaz/work/amphtml/build-system/tasks/runtime-test/helpers-unit.js:205:16)
    at getUnitTestsToRun (/Users/shihuaz/work/amphtml/build-system/tasks/runtime-test/helpers-unit.js:139:17)

The apparent reason is that https://www.npmjs.com/package/list-imports-exports is unable to parse https://github.com/ampproject/amphtml/blob/master/extensions/amp-story-auto-ads/0.1/story-ad-localization.js#L24

import xxx from xxx assert {type: "json"};

https://github.com/tc39/proposal-import-assertions this is where this syntax was founded I believe.

[powerivq]

https://github.com/ampproject/amphtml/pull/31615

[gmajoulet]

cc @jridgewell

[rsimha]

I believe this was fixed by #31619. When I tried editing extensions/amp-story-auto-ads/0.1/story-ad-localization.js and running gulp unit --local_changes --headless, the unit tests were correctly determined and executed.

@powerivq Can you sync to HEAD, run npm install, and retry this? If things are still broken, can you post info about your OS and environment?

[powerivq]

@rsimha thanks for looking! My branch https://github.com/powerivq/amphtml/commits/sticky-ad-imp includes https://github.com/ampproject/amphtml/pull/31619 but it still fails. It fails both on my macbook and travis https://travis-ci.com/github/ampproject/amphtml/builds/211124020

Can you try changing some CSS files like the one I did and see what happens?

[rsimha]

@powerivq Thanks for the links, I am able to reproduce the problem. You're correct in pointing out that the parser is tripping up on the JSX import assertion syntax. Here's what I found out while debugging:

• The list-imports-exports module is tripping up here in the parse() function while parsing story-ad-localization.js#L24.
• The parse() function accepts an optional extraPlugins argument, which we're currently not using.
• I tried passing in ['importAssertions'] as extraPlugins to parse(), but it didn't help.

diff --git a/build-system/tasks/runtime-test/helpers-unit.js b/build-system/tasks/runtime-test/helpers-unit.js
index 28f46f91f..8925bd1de 100644
--- a/build-system/tasks/runtime-test/helpers-unit.js
+++ b/build-system/tasks/runtime-test/helpers-unit.js
@@ -87,7 +87,8 @@ function extractCssJsFileMap() {
  */
 function getImports(jsFile) {
   const jsFileContents = fs.readFileSync(jsFile, 'utf8');
-  const {imports} = listImportsExports.parse(jsFileContents);
+  const extraPlugins = ['importAssertions'];
+  const {imports} = listImportsExports.parse(jsFileContents, extraPlugins);
   const files = [];
   const jsFileDir = path.dirname(jsFile);
   imports.forEach(function (file) {

@jridgewell Any other ideas?

[jridgewell]

Fixed with https://github.com/powerivq/amphtml/commit/e684689df0560e15bf2bee48f5d321a6529e970f. The @babel/parser version we were using was v7.11, which still uses the old with syntax instead of assert. Upgrading it (a bit annoying, because it's a transitive dep) to v7.12 and including the optional 'importAssertions' plugin fixes the issue.

Closing with https://github.com/powerivq/amphtml/commit/e684689df0560e15bf2bee48f5d321a6529e970f

[mszylkowski]

Did this get merged? I only see the issue got closed but no linked PR

[powerivq]

Not merged yet. It was squeezed into my original PR https://github.com/ampproject/amphtml/pull/31491 and is await merging.

[powerivq]

It was merged now.