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

fix(test): fix failing latitude test #2116

Merged
merged 3 commits into from
May 1, 2023

Conversation

matthewmayer
Copy link
Contributor

fix #2080

@matthewmayer matthewmayer requested a review from a team as a code owner May 1, 2023 10:13
@matthewmayer matthewmayer self-assigned this May 1, 2023
@matthewmayer matthewmayer added p: 1-normal Nothing urgent c: test m: location Something is referring to the location module labels May 1, 2023
@ST-DDT ST-DDT requested review from a team May 1, 2023 10:18
@codecov
Copy link

codecov bot commented May 1, 2023

Codecov Report

Merging #2116 (d41c2a6) into next (ec1ca12) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2116   +/-   ##
=======================================
  Coverage   99.59%   99.60%           
=======================================
  Files        2567     2567           
  Lines      243394   243394           
  Branches     1254     1255    +1     
=======================================
+ Hits       242420   242422    +2     
+ Misses        947      945    -2     
  Partials       27       27           
Impacted Files Coverage Δ
src/locales/en_AU_ocker/location/city_name.ts 100.00% <ø> (ø)
src/locales/en_NG/location/city_name.ts 100.00% <ø> (ø)
src/locales/en_ZA/location/city_name.ts 100.00% <ø> (ø)
src/locales/en_AU_ocker/location/city_pattern.ts 100.00% <100.00%> (ø)
src/locales/en_AU_ocker/location/index.ts 100.00% <100.00%> (ø)
src/locales/en_NG/location/city_pattern.ts 100.00% <100.00%> (ø)
src/locales/en_NG/location/index.ts 100.00% <100.00%> (ø)
src/locales/en_ZA/location/city_pattern.ts 100.00% <100.00%> (ø)
src/locales/en_ZA/location/index.ts 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented May 1, 2023

This doesn't really fix the issue, since those test failures come from number.float returning a floating point number where all decimal places are exactly 0.
For example faker.number.float({ precision: 0.001 }) CAN return a number like x.000 which JS trims to x. This is not a problem with the float implementation, but with how JS handles floating point number.

The fix you provided will have nicer error logs (at least I believe so) but doesn't actually fix the problem.

Ok, I read your comments in #2080 and saw that you were already aware of the actual problem. That made me curious and have an additional look at the chang you provided. I didn't see that the precision function was actually used to change the expected value. MB.

@ST-DDT ST-DDT enabled auto-merge (squash) May 1, 2023 21:24
@ST-DDT ST-DDT merged commit 7f9e9df into faker-js:next May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: test m: location Something is referring to the location module p: 1-normal Nothing urgent
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Test failure for latitude
4 participants