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

Addon tests frequently fail when running multi-process #799

Closed
bendemboski opened this issue Apr 30, 2021 · 4 comments
Closed

Addon tests frequently fail when running multi-process #799

bendemboski opened this issue Apr 30, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@bendemboski
Copy link
Contributor

If JOBS is not set to 1 and thread-loader is active, addon tests will frequently (but non-deterministically) fail complaining about missing imports for qunit and other dependencies that are only imported from the tests/dummy app.

Symptoms

Sometimes the entire test run will fail because of an error importing qunit from test-helper.js:

loader.js:247 Uncaught Error: Could not find module `qunit` imported from `(require)`
    at missingModule (loader.js:247)
    at findModule (loader.js:258)
    at requireModule (loader.js:24)
    at eval (qunit.js:1)
    at Object.../../../../../externals/qunit.js (chunk.b70187c243ba81f2007b.js:1136)
    at __webpack_require__ (chunk.b70187c243ba81f2007b.js:1200)
    at eval (test-helper.js:6)
    at Module../tests/test-helper.js (chunk.b70187c243ba81f2007b.js:898)
    at __webpack_require__ (chunk.b70187c243ba81f2007b.js:1200)
    at eval (test.js:702)

Sometimes test-helper.js' import will succeed, but individual test modules will fail to load with

Died on test #1     at TestLoader.moduleLoadFailure (webpack://dummy/../../../../node_modules/ember-qunit/test-loader.js?:37:40)
    at TestLoader.require (webpack://dummy/../../../../node_modules/ember-cli-test-loader/test-support/index.js?:77:12)
    at TestLoader.loadModules (webpack://dummy/../../../../node_modules/ember-cli-test-loader/test-support/index.js?:67:12)
    at loadTests (webpack://dummy/../../../../node_modules/ember-qunit/test-loader.js?:59:20)
    at start (webpack://dummy/../../../../node_modules/ember-qunit/index.js?:196:60)
    at eval (webpack://dummy/./tests/test-helper.js?:23:51)
    at Module../tests/test-helper.js (http://localhost:5601/assets/chunk.e0efb8a0a3f7216692f8.js:898:1)
    at __webpack_require__ (http://localhost:5601/assets/chunk.e0efb8a0a3f7216692f8.js:1190:42): Could not find module `qunit` imported from `(require)`

Source: | Error: Could not find module `qunit` imported from `(require)`
     at missingModule (http://localhost:5601/assets/vendor.js:256:11)
     at findModule (http://localhost:5601/assets/vendor.js:267:7)
     at requireModule (http://localhost:5601/assets/vendor.js:33:15)

Proximate cause

The proximate cause of this is that certain modules will end up trying to import qunit as an external:

var _<tempstuff>_embroider_externals_qunit__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__("../../../../../externals/qunit.js");

Which modules this happen to is non-deterministic, although the above results get saved to the babel loader cache ($TMPDIR/embroider/webpack-babel-loader), so barring clearing the cache or making changes to the source files, it will look deterministic, but it's really just a tainted cache.

This, in turn, is happening because in a multi-process build environment, in some of the thread-loader worker processes, when this code runs on files in the tests/dummy app, the dummy app is not in the root package cache, so we think that the files are part of the addon itself rather than the addon's dummy app. Since qunit (and potentially other libraries as well) are only installed in the dummy app, and not the addon, we fail to resolve them relative to the addon and mark them as external.

Root cause

From conversations with @ef4, the root cause is that the code that runs in the worker processes when in a multi-process build environment relies on the package cache already having been imperatively seeded, but that seeding only happens in the main build process, not the thread-loader worker processes.

Further data

After doing a whole bunch of console.log() debugging, it appears that the non-determinism is related to the order that babel plugins are invoked in the worker process. Anytime template-colocation-plugin is invoked on a file that lives in the tests/dummy app, it will add the dummy app to the root package cache if it's not already added. Anytime babel-plugin-adjust-imports runs on such a file, it will not add the dummy app to the root package cache, so if the dummy app is already in the cache, the plugin will resolve things correctly, but if the dummy app is not in the cache, it will think the test file is part of the addon and resolve any imports of test-only packages as externals. Hence, what determines whether the adjust imports plugin will do the wrong thing or not is whether the colocation plugin has previously run on a test file in that worker process, and that's non-deterministic.

I am somewhat stumped as to why template-colocation-plugin is able to correctly add the dummy app to the cache when it calls PackageCache.ownerOfFile(), but babel-plugin-adjust-imports is not. This is a sample stack trace of template-colocation-plugin adding it:

    at /Users/ben/projects/unity/node_modules/@embroider/shared-internals/src/package-cache.js:57:18
    at Object.getOrCreate (/Users/ben/projects/unity/node_modules/@embroider/shared-internals/src/get-or-create.js:7:18)
    at PackageCache.get (/Users/ben/projects/unity/node_modules/@embroider/shared-internals/src/package-cache.js:56:33)
    at PackageCache.ownerOfFile (/Users/ben/projects/unity/node_modules/@embroider/shared-internals/src/package-cache.js:79:25)
    at PluginPass.enter (/Users/ben/projects/unity/node_modules/@embroider/core/src/template-colocation-plugin.js:21:54)
    at newFn (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/visitors.js:175:21)
    at NodePath._call (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/path/context.js:55:20)
    at NodePath.call (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/path/context.js:42:17)
    at NodePath.visit (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/path/context.js:92:31)
    at TraversalContext.visitQueue (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/context.js:116:16)
    at TraversalContext.visitSingle (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/context.js:85:19)
    at TraversalContext.visit (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/context.js:144:19)
    at Function.traverse.node (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/index.js:82:17)
    at traverse (/Users/ben/projects/embroider/node_modules/@babel/traverse/lib/index.js:62:12)
    at transformFile (/Users/ben/projects/embroider/node_modules/@babel/core/lib/transformation/index.js:107:29)
    at transformFile.next (<anonymous>)
    at run (/Users/ben/projects/embroider/node_modules/@babel/core/lib/transformation/index.js:35:12)
    at run.next (<anonymous>)
    at Function.transform (/Users/ben/projects/embroider/node_modules/@babel/core/lib/transform.js:19:41)
    at transform.next (<anonymous>)
    at step (/Users/ben/projects/embroider/node_modules/gensync/index.js:261:32)
    at /Users/ben/projects/embroider/node_modules/gensync/index.js:273:13
    at async.call.result.err.err (/Users/ben/projects/embroider/node_modules/gensync/index.js:223:11)
    at /Users/ben/projects/embroider/node_modules/gensync/index.js:189:28
    at /Users/ben/projects/embroider/node_modules/@babel/core/lib/gensync-utils/async.js:62:7
    at /Users/ben/projects/embroider/node_modules/gensync/index.js:113:33
    at step (/Users/ben/projects/embroider/node_modules/gensync/index.js:287:14)
    at /Users/ben/projects/embroider/node_modules/gensync/index.js:273:13
    at async.call.result.err.err (/Users/ben/projects/embroider/node_modules/gensync/index.js:223:11)

For some reason this call results in correctly adding the dummy app to the cache, while this call (invoked from here) does not.

Workaround

A workaround is to run with JOBS=1 to disable thread-loader and run everything in one process, so the imperative seeding happens in the same process in which the loaders/plugins run. However, you will also likely need to delete $TMPDIR/embroider/webpack-babel-loader, which will have cached versions of transpiled files with incorrect import resolutions.

@rwjblue rwjblue added the bug Something isn't working label Apr 30, 2021
bendemboski added a commit to bendemboski/ember-add-listener-helper that referenced this issue May 5, 2021
@mydea
Copy link
Contributor

mydea commented May 10, 2021

FYI, also running into this with an addon. For me, deleting /tmp/embroider and running JOBS=1 ember t does not fix the issue.

bendemboski added a commit to bendemboski/ember-add-listener-helper that referenced this issue May 10, 2021
@ef4
Copy link
Contributor

ef4 commented Jul 5, 2021

I think this may have been fixed by #881

@angelayanpan
Copy link
Collaborator

hi, just wondering if this is fixed? @bendemboski

@bendemboski
Copy link
Contributor Author

Yes, it has! I've been running without the JOBS=1 workaround for some time now and haven't encountered this issue again, I just forgot to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants