test(locale): check for duplicated entries #1137

xDivisionByZerox · 2022-07-08T23:00:47Z

This PR will introduce a test that checks every LocaleDefinition for duplicated entries.

The most important file that needs review is test/locales.spec.ts. Everything else are just changes that are required by this test case.

xDivisionByZerox · 2022-07-08T23:02:39Z

I expect the test pipeline to fail for demonstration purposes.

@ST-DDT would you have a look at the implementation and the error log in general? I trust in you and your abilities on such things.

ST-DDT · 2022-07-09T00:08:45Z

The implementation looks good, maybe you could use this expectation:

expect(
  uniqueDuplication,
  `Found duplicate entries: [${uniqueDuplication
    .sort()
    .join(', ')}]`
).toBe([]);
// The sort should probably be outside of the string template

The toBe check focuses more on the content of the array than the length.

What do you think?

(PS: We have to check if there are/should be exceptions to this rule)

test/locales.spec.ts

xDivisionByZerox · 2022-07-09T13:01:27Z

So I need to change about 1,4k locale files since they contain duplicates. Is it reasonable to fix all of this in one (THIS) PR? My current workflow is:

sort all entries in a locale files
commit with message 'chore(locales): *localeName* *fileName* sort alphabetically'
remove duplicated entries
commit with message 'chore(locales): *localeName* *fileName* remove duplicates'

My question: Should I provide an additional PR where I sort ALL locale files alphabetically first. This could be much clearer for the git history.

I use the Tyriar.sort-lines VS Code extension to sort locale file entries.

Shinigami92 · 2022-07-09T13:11:05Z

@xDivisionByZerox yes, for me that sounds more safe

PR for sort
PR for dedup

That way we have a better commit history in main branch

Co-authored-by: ST-DDT <[email protected]>

codecov · 2022-09-25T21:09:35Z

Codecov Report

Merging #1137 (f999ba9) into main (f0b60b9) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head f999ba9 differs from pull request most recent head e418402. Consider uploading reports for the commit e418402 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1137      +/-   ##
==========================================
- Coverage   99.63%   99.61%   -0.02%     
==========================================
  Files        2165     2165              
  Lines      241432   237194    -4238     
  Branches     1021     1017       -4     
==========================================
- Hits       240549   236287    -4262     
- Misses        862      886      +24     
  Partials       21       21

Impacted Files	Coverage Δ
src/locales/af_ZA/name/female_first_name.ts	`100.00% <ø> (ø)`
src/locales/af_ZA/name/first_name.ts	`100.00% <ø> (ø)`
src/locales/af_ZA/name/last_name.ts	`100.00% <ø> (ø)`
src/locales/af_ZA/name/male_first_name.ts	`100.00% <ø> (ø)`
src/locales/ar/address/city_name.ts	`100.00% <ø> (ø)`
src/locales/ar/commerce/department.ts	`100.00% <ø> (ø)`
src/locales/ar/name/female_first_name.ts	`100.00% <ø> (ø)`
src/locales/ar/name/last_name.ts	`100.00% <ø> (ø)`
src/locales/ar/phone_number/formats.ts	`100.00% <ø> (ø)`
src/locales/ar/team/creature.ts	`100.00% <ø> (ø)`
... and 213 more

ST-DDT

The check itself looks good, but duplicated. You might want to want to move the "describe if array ..." block into a helper method allowing recursive calls to it.
I could provide a patch/diff, if my explanation isn't easy to understand.

test/locales.spec.ts

ST-DDT · 2022-09-25T23:00:58Z

It looks like this PR doesn't really reduce the line count: +10,463 -4,859
(not that it is important)

Shinigami92 · 2022-09-26T06:36:37Z

It looks like this PR doesn't really reduce the line count: +10,463 -4,859 (not that it is important)

Yeah, you are right
Somehow some files are generated with fully new content 🤔
Example:

xDivisionByZerox · 2022-09-26T07:43:47Z

Somehow some files are generated with fully new content 🤔

Oh damn. Good catch. I will revert those files. I guess they slipped through as I played with the generate-locales script.

…faker-js/faker into test/locales/duplicated-entries

xDivisionByZerox · 2022-09-26T18:20:59Z

Those "additions" should be reverted now. They happened due to those files being implemented in a dynamic/logical way, not with a static data set.

ST-DDT · 2022-09-26T18:40:24Z

@xDivisionByZerox I found a way to simplify the test a bit, feel free to merge: diff

ST-DDT · 2022-09-29T21:28:58Z

@xDivisionByZerox Did you see my comment here: #1137 (comment)

src/locales/en/name/title.ts

src/locales/en_NG/address/city_prefix.ts

xDivisionByZerox added c: feature Request for new feature c: test labels Jul 8, 2022

xDivisionByZerox added this to the v7 - Current Major milestone Jul 8, 2022

xDivisionByZerox requested a review from ST-DDT July 8, 2022 23:00

xDivisionByZerox self-assigned this Jul 8, 2022

ST-DDT reviewed Jul 9, 2022

View reviewed changes

test/locales.spec.ts Outdated Show resolved Hide resolved

xDivisionByZerox changed the title ~~test(locales): check for duplicated entries~~ WIP: test(locales): check for duplicated entries Jul 9, 2022

xDivisionByZerox requested a review from a team July 9, 2022 12:55

Shinigami92 changed the title ~~WIP: test(locales): check for duplicated entries~~ WIP: test(locale): check for duplicated entries Aug 12, 2022

xDivisionByZerox and others added 17 commits September 23, 2022 18:40

test(locales): check for duplicated entries

86b2631

test(locales): reword no-duplicates description

c9d7a37

Co-authored-by: ST-DDT <[email protected]>

test(locales): no-duplicates change expect statement

32f2f8f

chore(locales): zu_ZA last_name sort alphabetically

c0c0626

chore(locales): zu_ZA last_name remove duplicates

62fded2

chore(locales): zu_ZA first_name sort alphabetically

ec94598

chore(locales): zu_ZA first_name remove duplicates

7609dd3

chore(locales): zu_ZA female_first_name sort alphabetically

b3c99e5

chore(locales): zu_ZA female_first_name remove duplicates

5559ab0

chore(locales): zh_TW first_name sort alphabetically

88ac503

chore(locales): zh_TW first_name remove duplicates

d06dbe0

chore(locales): zh_CH first_name sort alphabetically

86bd85a

chore(locales): zh_CH first_name remove duplicates

596546e

chore(locales): zh_CH city_suffix sort alphabetically

3ac841f

chore(locales): zh_CH city_suffix remove duplicates

2e04e5f

chore(locales): zh_CH city_prefix sort alphabetically

9df82f0

chore(locales): zh_CH city_prefix remove duplicates

29fed2c

xDivisionByZerox added s: accepted Accepted feature / Confirmed bug c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs c: locale Permutes locale definitions labels Sep 25, 2022

ST-DDT reviewed Sep 25, 2022

View reviewed changes

test/locales.spec.ts Outdated Show resolved Hide resolved

xDivisionByZerox and others added 4 commits September 26, 2022 19:39

test(locales): simplify duplication tests

aa4a692

Merge branch 'main' into test/locales/duplicated-entries

f364dbc

refactor(locales): revert overwritten logic implementation

2b90720

Merge branch 'test/locales/duplicated-entries' of https://github.com/…

43d717f

…faker-js/faker into test/locales/duplicated-entries

refactor(locale): unique name combinations

3c31ef7

ST-DDT previously approved these changes Sep 29, 2022

View reviewed changes

Shinigami92 reviewed Oct 1, 2022

View reviewed changes

src/locales/en/name/title.ts Show resolved Hide resolved

src/locales/en_NG/address/city_prefix.ts Show resolved Hide resolved

refactor(locale): remove undesirable values form en_NG city_prefix

f999ba9

xDivisionByZerox dismissed ST-DDT’s stale review via f999ba9 October 6, 2022 08:22

Shinigami92 approved these changes Oct 6, 2022

View reviewed changes

ST-DDT approved these changes Oct 6, 2022

View reviewed changes

ejcheng approved these changes Oct 6, 2022

View reviewed changes

ST-DDT enabled auto-merge (squash) October 8, 2022 10:47

Merge branch 'main' into test/locales/duplicated-entries

e418402

ST-DDT merged commit 5dc8f0e into main Oct 8, 2022

Shinigami92 deleted the test/locales/duplicated-entries branch October 8, 2022 14:02

xDivisionByZerox mentioned this pull request Oct 11, 2022

test: fix lint warning and simplify locales test #1430

Merged

wael-fadlallah pushed a commit to wael-fadlallah/faker that referenced this pull request Oct 14, 2022

test(locale): check for duplicated entries (faker-js#1137)

d1fed7d

ST-DDT mentioned this pull request Jan 19, 2023

Proposal: test for duplicate local definition values #975

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(locale): check for duplicated entries #1137

test(locale): check for duplicated entries #1137

xDivisionByZerox commented Jul 8, 2022 •

edited

Loading

xDivisionByZerox commented Jul 8, 2022

ST-DDT commented Jul 9, 2022 •

edited

Loading

xDivisionByZerox commented Jul 9, 2022

Shinigami92 commented Jul 9, 2022 •

edited

Loading

codecov bot commented Sep 25, 2022 •

edited

Loading

ST-DDT left a comment •

edited

Loading

ST-DDT commented Sep 25, 2022

Shinigami92 commented Sep 26, 2022

xDivisionByZerox commented Sep 26, 2022

xDivisionByZerox commented Sep 26, 2022

ST-DDT commented Sep 26, 2022 •

edited

Loading

ST-DDT commented Sep 29, 2022

test(locale): check for duplicated entries #1137

test(locale): check for duplicated entries #1137

Conversation

xDivisionByZerox commented Jul 8, 2022 • edited Loading

xDivisionByZerox commented Jul 8, 2022

ST-DDT commented Jul 9, 2022 • edited Loading

xDivisionByZerox commented Jul 9, 2022

Shinigami92 commented Jul 9, 2022 • edited Loading

codecov bot commented Sep 25, 2022 • edited Loading

Codecov Report

ST-DDT left a comment • edited Loading

Choose a reason for hiding this comment

ST-DDT commented Sep 25, 2022

Shinigami92 commented Sep 26, 2022

xDivisionByZerox commented Sep 26, 2022

xDivisionByZerox commented Sep 26, 2022

ST-DDT commented Sep 26, 2022 • edited Loading

ST-DDT commented Sep 29, 2022

xDivisionByZerox commented Jul 8, 2022 •

edited

Loading

ST-DDT commented Jul 9, 2022 •

edited

Loading

Shinigami92 commented Jul 9, 2022 •

edited

Loading

codecov bot commented Sep 25, 2022 •

edited

Loading

ST-DDT left a comment •

edited

Loading

ST-DDT commented Sep 26, 2022 •

edited

Loading