azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.05k stars 155 forks source link

Added Holiday Provider for Brazil #21

Closed dorianneto closed 8 years ago

stelgenhof commented 8 years ago

Thank you @dorianneto ! Very nice, however it has some conflicts with the current branch. Can you check again?

dorianneto commented 8 years ago

Checked and validated :)

I'm sorry for a lot of commits. I forgot to use the php-cs-fixer before make push :P

dorianneto commented 8 years ago

@stelgenhof ?

stelgenhof commented 8 years ago

@dorianneto Not sure what you mean, though :) I still need to check your PR; planning this for the 1.4 version. Once I have finished 1.3 I will merge it into the master branch. In case you need it earlier, you can still use your own fork.

dorianneto commented 8 years ago

All done :)

stelgenhof commented 8 years ago

Thanks @dorianneto ! Looking good. Let me review once more and merge it soon. Appreciate the PR!

stelgenhof commented 8 years ago

Just ran another unittest and found some errors:

1) Yasumi\tests\Brazil\BrazilTest::testNationalHolidays
Failed asserting that an array has the key 'tiradentesDay'.

If a holidays is of a certain type (e.g. National), make sure they are added in the respective test of the BrazilTest class.

Running a single iteration of PHPUnit might not detect all issues. Please run with --repeat 10 for example. You might see more issues appearing.

dorianneto commented 8 years ago

Before push this commit, I verified all unittests and not found no problem. Now, I ran with --repeat 10 and sometimes have a problem sometimes not.

This is a PHPUnit bug or I have to do anything? I never experienced this problem, sorry :(

stelgenhof commented 8 years ago

The reason for that is, that the unit tests use a randomized year. It might be possible that for that particular year the respective holiday doesn't exist. Usually this becomes apparent when running multiple iterations :). Nothing to worry about. Let me merge it and fix it for you.