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

[Qol] Load i18n en locales during tests #4553

Merged

Conversation

flx-sta
Copy link
Collaborator

@flx-sta flx-sta commented Oct 3, 2024

Why am I making these changes?

Right now it's kinda hard to read the obfuscated logs during tests as it's just i18n keys.

What are the changes from a developer perspective?

  • Added a MSW that handles loading the locales files during tests
  • Fix all tests that were written in a way that they were affected by this. They shouldn't anymore in the future now.

Screenshots/Videos

N/A

How to test the changes?

Run the tests. YOu should see human readable english output yet fully functional tests.

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • [ ] Have I considered writing automated tests for the issue?
  • [ ] If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • [] Are the changes visual?

the backend is being supported by using msw which will import the correct file from the local locales folder
@flx-sta flx-sta added Tests Automated tests related Refactor Rewriting existing code related labels Oct 3, 2024
@flx-sta flx-sta marked this pull request as ready for review October 3, 2024 03:22
@flx-sta flx-sta requested a review from a team as a code owner October 3, 2024 03:22
DayKev
DayKev previously approved these changes Oct 3, 2024
MokaStitcher
MokaStitcher previously approved these changes Oct 3, 2024
@MokaStitcher
Copy link
Collaborator

don't know if this is just normal variance because the sample is small, but test runs with this seem to take longer on average than the other runs

seem to take ~3mins on average
Screenshot 2024-10-03 at 16 40 13
vs for other PRs it seems to be more around 2m15-2m30
Screenshot 2024-10-03 at 16 40 42

Not sure if much can be done about this?

@flx-sta
Copy link
Collaborator Author

flx-sta commented Oct 3, 2024

don't know if this is just normal variance because the sample is small, but test runs with this seem to take longer on average than the other runs

seem to take ~3mins on average (image) vs for other PRs it seems to be more around 2m15-2m30 (image)

Not sure if much can be done about this?

Not really. It's the nature of allowing requests to be sent and having to wait for them.
Here is the 3 states:

  • before lazy load: All the locales were there ready with the build
  • after lazy-load (now): i18next was mocked away completely so less to build and less to load
  • this PR: i18next is allowed to send it's requests and they are redirected by MSW towards loading the file. This causes an import() which is an async "request". MSW is generally much faster than an actually request but there is just so much a machine can do

Copy link
Collaborator

@torranx torranx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few typos, would love to see more description on the PR, otherwise looks good

global.d.ts Outdated Show resolved Hide resolved
src/test/system/game_data.test.ts Outdated Show resolved Hide resolved
DayKev
DayKev previously approved these changes Oct 4, 2024
@DayKev DayKev dismissed stale reviews from MokaStitcher and themself via 0f49c53 October 4, 2024 05:39
Co-authored-by: Adrian T. <[email protected]>
DayKev
DayKev previously approved these changes Oct 4, 2024
torranx
torranx previously approved these changes Oct 4, 2024
update reference to `56eeb809eb5a2de40cfc5bc6128a78bef14deea9` (from `3ccef8472dd7cc7c362538489954cb8fdad27e5f`)
@flx-sta flx-sta dismissed stale reviews from torranx and DayKev via 4425243 October 5, 2024 00:39
@DayKev DayKev requested a review from torranx October 8, 2024 03:12
@flx-sta flx-sta merged commit f180b60 into pagefaultgames:beta Oct 9, 2024
13 checks passed
@flx-sta flx-sta deleted the qol/load-i18n-en-locales-during-test branch October 9, 2024 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Rewriting existing code related Tests Automated tests related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants