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

Breaking test: generates one plural key for unkown languages #1069

Open
jmlavoier opened this issue Sep 12, 2024 · 1 comment
Open

Breaking test: generates one plural key for unkown languages #1069

jmlavoier opened this issue Sep 12, 2024 · 1 comment

Comments

@jmlavoier
Copy link

🐛 Bug Report

Running the yarn test

parser
    options
      1) generates one plural key for unknow languages


  0 passing (11ms)
  1 failing

  1) parser
       options
         generates one plural key for unknow languages:

      Uncaught AssertionError: expected { 'test {{count}}_one': '', …(2) } to deeply equal { 'test {{count}}_one': '', …(1) }
      + expected - actual

       {
      -  "test {{count}}_many": ""
         "test {{count}}_one": ""
         "test {{count}}_other": ""
       }

To Reproduce

$ yarn
$ yarn test

Expected behavior

Taking into consideration that I haven't changed anything in the codebase and I'm running it on the master branch, which is supposed to be the production-ready version, the test should run successfully.

 272 passing (18s)
  0 failing

Your Environment

  • runtime version: node v18
  • i18next version: ^23.5.1
  • i18next-parser version: 9.0.2
  • yarn version:: 1.22.22
  • os: Mac (M1)
@jmlavoier
Copy link
Author

jmlavoier commented Sep 13, 2024

Grooming

Three points to consider to solve this problem.

  1. The problem is on i18next in node environment
  2. The problem is on the test
  3. The problem is that we're doing a wrong test, testing the result of i18next.

1. The problem is on i18next in node environment

I've tested the i18next in node environment and the browser and got different results. I realized that the many possibility is coming only on node environment.

I.e:

This simple implementation got different results:

import i18next from "i18next";

i18next.init({
  pluralSeparator: "_",
  nsSeparator: ":",
});

i18next.services.pluralResolver
  .getSuffixes("unknown", { ordinal: undefined })
  .forEach((suffix) => {
    console.log("suffix", suffix);
  });

Browser (Brave: v1.69.162 - Chromium 128.0.6613.120)

suffix:  _one
suffix:  _other

In this case, the test should not fail.

Node v18.19.1

suffix _one
suffix _many
suffix _other

Due to the many the test is failing

For both tests I used i18next v23.5.1

So if there is a compatibility issue, maybe it's necessary identify what's needs to be fixed to make it consistent on both environments. I know that it should not be executed on browser, but considering that the test is breaking in node environment that is the more common to be executed.

2. The problem is on the test

Considering that the many is not a problem, we would change the test to accept the many.

i18nextParser.once('end', () => {
        assert.deepEqual(result, {
          'test {{count}}_one': '',
          'test {{count}}_many': '', // <- Add this to the assert 
          'test {{count}}_other': '',
        })
        done()
      })

3. The problem is that we're doing a wrong test, testing the result of i18next.

Finally, we should question whether this test is necessary. Since this behavior is part of the i18next library, testing it here introduces unnecessary dependency on external behavior. The responsibility of pluralization lies with i18next, not our library. If there is an issue with i18next, the problem should be addressed within that library, not through our tests.

Conclusion

Let's discuss about these points and decide which solution fits better to this issue so that I can create a PR to sort it out.

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

No branches or pull requests

1 participant