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

Specified the locale in setupIntl() #10464

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

ijlee2
Copy link
Contributor

@ijlee2 ijlee2 commented Jun 5, 2024

In ember-intl@v7, the test helper setupIntl() requires end-developers to specify a locale. I updated the blueprint file for tests/helpers/index.ts to reflect this breaking change.

Note

The change to tests/helpers/index.ts is backward-compatible, because specifying the locale is optional in earlier versions of ember-intl.

Two possible alternatives:

  1. Remove the commented line from setupApplicationTest(), because the test helper setupIntl() is really meant to help end-developers write rendering and unit tests.
  2. Move the commented line to setupRenderingTest(), in case an end-developer wants an example of calling a function (e.g. setupIntl()) in all or most rendering tests.

@ijlee2 ijlee2 force-pushed the update-test-helpers-index branch 2 times, most recently from 22666a5 to 985eabe Compare June 11, 2024 14:49
@ef4
Copy link
Contributor

ef4 commented Jun 18, 2024

I think the test failures here are caused by an ember-data beta release problem that has been fixed, not sure if it's been released.

This PR seems fine and should merge whenever we can get CI green again. We do need to confirm that no tests are failing just because of the blueprint changes.

@kategengler
Copy link
Member

@ijlee2 Can you try rebasing to see if that fixes CI?

@ijlee2 ijlee2 force-pushed the update-test-helpers-index branch from 985eabe to e5ce991 Compare June 25, 2024 16:52
@ijlee2
Copy link
Contributor Author

ijlee2 commented Jun 25, 2024

@kategengler Thanks for letting me know. The branch had already been rebased, after which CI had failed, so I cherry-picked the commit to re-trigger CI just now.

@kategengler
Copy link
Member

I'm happy to merge now but do you want to retarget to beta so it makes it into 5.10?

@ijlee2 ijlee2 force-pushed the update-test-helpers-index branch from e5ce991 to 1d2c3c2 Compare June 26, 2024 05:06
@ijlee2 ijlee2 changed the base branch from master to beta June 26, 2024 05:07
@ijlee2
Copy link
Contributor Author

ijlee2 commented Jun 26, 2024

I rebased the branch against v5.10.0-beta.0 and changed the target branch to beta. I hope it's right; if not, feel free to change the pull request, as I'll be out for several days. Thanks.

@ijlee2 ijlee2 force-pushed the update-test-helpers-index branch from 1d2c3c2 to e4d0d38 Compare June 26, 2024 06:42
@kategengler kategengler merged commit 9654f28 into ember-cli:beta Jun 26, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants