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

Leverage test-scenarios from ember-auto-import #756

Merged
merged 2 commits into from
Apr 17, 2021

Conversation

thoov
Copy link
Collaborator

@thoov thoov commented Apr 7, 2021

Problem

Currently our test suite is setup that each test package is an actual ember application (or addon). While this works, we incur a large overhead in maintenance, dryness, and complexity.

Solution

Instead of individual packages that are full blown applications, we can leverage a shared base "template" that we apply diffs on top. This base template can be maintained via ember-cli-update while all of the test scenarios don't need to care about the underlining template.

Future

This PR simply converts the fastboot-app test package over to the new format. Once this lands all the rest of the test pacakges can be converted one by one.

@ef4 ef4 mentioned this pull request Apr 12, 2021
ef4 added a commit that referenced this pull request Apr 12, 2021
This will eventually get expanded to a matrix when #756 lands, but I need to test some newer stuff now.
@ef4
Copy link
Contributor

ef4 commented Apr 16, 2021

Thank you for fighting the good fight here.

@thoov thoov marked this pull request as ready for review April 16, 2021 02:20
@thoov thoov changed the title WIP: Leverage test-scenarios from ember-auto-import Leverage test-scenarios from ember-auto-import Apr 16, 2021
@thoov thoov force-pushed the new-test-suite branch 2 times, most recently from a33a6fd to d463ea5 Compare April 16, 2021 08:23
@lifeart
Copy link
Collaborator

lifeart commented Apr 16, 2021

@thoov
Copy link
Collaborator Author

thoov commented Apr 16, 2021

@ef4 / @rwjblue:

Ok this should be good to go now. The node_modules hoisting issue was just a bad yarn.lock (so I don't need to do the yarn hack). I also had to add @ember/string to a few packages to work around: emberjs/data#6791

This only adds release as scenarios so we can discuss what the full matrix should look like. Additionally, an improvement to scenario-tester would be to add matrix multiplication such that we can have environment variables that are different on each scenario (such as CLASSIC vs EMBROIDER)

@ef4
Copy link
Contributor

ef4 commented Apr 17, 2021

Thanks, looking good.

an improvement to scenario-tester would be to add matrix multiplication such that we can have environment variables that are different on each scenario

Yeah the closest example is this plus this which lets you multiply out the scenarios, but not bake the environment variables right into the scenarios themselves.

@ef4 ef4 merged commit 8b2c20d into embroider-build:master Apr 17, 2021
@ef4 ef4 added the internal label Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants