Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Kz faker random wrong output from getCenturyByYear #2026

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maris-liepins-ermac
Copy link

@maris-liepins-ermac maris-liepins-ermac commented Jul 9, 2020

If $year is set to 2001, there is a chance that DateTime::year() will return value less than 2000, which will cause a bug and instead of 21st century will return 20th century. DateTime::year() returns random value.

#2025

Use current year to search for century instead of random year.
@codecov-commenter
Copy link

Codecov Report

Merging #2026 into master will decrease coverage by 0.70%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2026      +/-   ##
============================================
- Coverage     56.56%   55.86%   -0.71%     
  Complexity     2068     2068              
============================================
  Files           306      306              
  Lines          4849     4849              
============================================
- Hits           2743     2709      -34     
- Misses         2106     2140      +34     
Impacted Files Coverage Δ Complexity Δ
src/Faker/Provider/kk_KZ/Person.php 82.14% <100.00%> (ø) 12.00 <0.00> (ø)
src/Faker/Provider/de_AT/Person.php 0.00% <0.00%> (-100.00%) 1.00% <0.00%> (ø%)
src/Faker/Provider/de_DE/Person.php 0.00% <0.00%> (-100.00%) 1.00% <0.00%> (ø%)
src/Faker/Provider/ne_NP/Person.php 0.00% <0.00%> (-50.00%) 2.00% <0.00%> (ø%)
src/Faker/Provider/en_AU/PhoneNumber.php 0.00% <0.00%> (-50.00%) 2.00% <0.00%> (ø%)
src/Faker/Provider/vi_VN/Person.php 20.00% <0.00%> (-40.00%) 5.00% <0.00%> (ø%)
src/Faker/Provider/ko_KR/Address.php 60.00% <0.00%> (-40.00%) 5.00% <0.00%> (ø%)
src/Faker/Provider/en_HK/Address.php 33.33% <0.00%> (-33.34%) 9.00% <0.00%> (ø%)
src/Faker/Provider/sr_RS/Address.php 66.66% <0.00%> (-33.34%) 3.00% <0.00%> (ø%)
src/Faker/Provider/uk_UA/Company.php 28.57% <0.00%> (-28.58%) 3.00% <0.00%> (ø%)
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5337ce5...709303b. Read the comment docs.

@maris-liepins-ermac maris-liepins-ermac marked this pull request as ready for review July 9, 2020 12:58
@maris-liepins-ermac maris-liepins-ermac changed the title Update Person.php Kz faker random wrong output from getCenturyByYear Jul 9, 2020
@@ -183,7 +183,7 @@ class Person extends \Faker\Provider\Person
*/
private static function getCenturyByYear($year)
{
if ($year >= 2000 && $year <= DateTime::year()) {
if ($year >= 2000 && $year <= date('Y')) {
Copy link

Choose a reason for hiding this comment

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

why dont delete && $year <= date('Y') ?

Copy link

@jevgenijsblaus jevgenijsblaus Sep 14, 2020

Choose a reason for hiding this comment

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

To not run into a future I believe. We don't know why this statement exists, but I believe author know what is it for.
The only thing @maris-liepins-ermac did is switching from "Faker" DateTime with random year output to real DateTime year output

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants