embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
330 stars 137 forks source link

Use Vite for all tests #1840

Closed mansona closed 2 months ago

ef4 commented 2 months ago

The merge from main above fixed part of the failures in canary-static-app because

The above commit fixed the development mode canary-static-app. The next thing we debugged is the production mode canary-static-app. Before, that was doing EMBER_ENV=production ember test, which put ember into production mode while still including the tests in the build. Now the build happens separately from ember test so there's nothing telling the build to include tests, so they're not in dist.

Other issues identified while debugging (some of which only appear if you try to run one of the scenarios in vite dev, particularly with staticAddonsTrees===false):

We may want to consider not supporting staticAddonsTrees===false at all in the new major. It often introduces bugs that are otherwise not present. It's trying to give behavior closer to classic, but it can hurt more than it helps.

We also started debugging the macro-tests failure, which is again a staticAddonsTrees===false issue where the implicit-modules appears to have a module cycle involving @ember/modifier/index.

ef4 commented 2 months ago

Leading edge of work:

ef4 commented 2 months ago

I figured out the @ember/test-waiters issue. It's not a bug in embroider -- the addon actually includes a javascript module containing only export {}. Vite's dep optimizer doesn't handle that correctly.

(Arguably the addon shouldn't include such a module. It seems to exist as a workaround for other code doing a poor job of distinguishing type-only imports.)

ef4 commented 2 months ago

Down to only the watch mode tests.

I started reimplementing them to use a direct fetch for a given module, but that is going to be problematic. Vite controls rebuilds by propagating query params downward through the module graph. To get a realistic result we want to follow those same imports. This is a good fit for the audit assertions, but we'll need to extend those to be able to follow URLs and not just files.