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

test: properly convert function name to locale entry name #3197

Closed
wants to merge 3 commits into from

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Oct 19, 2024

Currently, the animal tests use the function name as the name of the entry.
This however doesn't work when the name uses camelCase such as petName.

This PR changes the locale data lookup to convert the camelCase function name to a snake_case locale entry name before using it.

Required for

Alternative

@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: test m: animal Something is referring to the animal module labels Oct 19, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Oct 19, 2024
@ST-DDT ST-DDT requested review from a team October 19, 2024 12:18
@ST-DDT ST-DDT self-assigned this Oct 19, 2024
Copy link

netlify bot commented Oct 19, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit c11488e
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/67196b75dee08100086ad6a8
😎 Deploy Preview https://deploy-preview-3197.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ST-DDT ST-DDT changed the title test: convert function name to entry name properly test: properly convert function name to entry name Oct 19, 2024
Copy link

codecov bot commented Oct 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (e3858f2) to head (c11488e).
Report is 3 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #3197      +/-   ##
==========================================
- Coverage   99.96%   99.95%   -0.01%     
==========================================
  Files        2797     2797              
  Lines      216762   216762              
  Branches      579      581       +2     
==========================================
- Hits       216686   216675      -11     
- Misses         76       87      +11     

see 1 file with indirect coverage changes

@ST-DDT ST-DDT changed the title test: properly convert function name to entry name test: properly convert function name to locale entry name Oct 19, 2024
@matthewmayer
Copy link
Contributor

matthewmayer commented Oct 19, 2024

Would it be better just to explicitly test each method in turn? Its repetitive but maybe easier to understand. Most other simple modules, even ones which are just helpers.arrayElement wrappers like faker.book just test every method one by one.

@matthewmayer
Copy link
Contributor

Would it be better just to explicitly test each method in turn? Its repetitive but maybe easier to understand. Most other simple modules, even ones which are just helpers.arrayElement wrappers like faker.book just test every method one by one.

for comparison i made #3198 - i find this easier to read and more consistent with other modules.

@ST-DDT
Copy link
Member Author

ST-DDT commented Oct 19, 2024

for comparison i made #3198 - i find this easier to read and more consistent with other modules.

Thanks for creating the PR for comparison.
IMO the current loop based version is easier/faster to fully understand, but I don't have a strong opinion either way.

@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Oct 23, 2024
@ST-DDT ST-DDT removed the s: needs decision Needs team/maintainer decision label Oct 26, 2024
@ST-DDT ST-DDT closed this Oct 26, 2024
@ST-DDT ST-DDT deleted the test/animal/snake_case branch October 26, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: test m: animal Something is referring to the animal module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants