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

Use gh action ci cd #404

Closed
wants to merge 22 commits into from
Closed

Conversation

s-weigand
Copy link
Contributor

@s-weigand s-weigand commented Dec 7, 2020

Here is the long ago promised PR to change the CI-CD from travis to github actions.

As mentioned in #341 I also implemented some other tooling changes:

  • Added pre-commit
  • Added autoformatting with black via pre-commit
  • Changed flake8 linting to run via pre-commit
  • Added tools to check *.rst files
  • Changed test runner to be pytest
  • Changed structure of tests, now the tests structure mirrors the structure of the project (IMHO makes it much easier to find the tests you are looking for)
  • Changed contributing documentation to reflect changed tooling

Looking at some changes black made (e.g. for holidays/countries/hongkong.py), maybe a different formatter which is more configurable (e.g. yapf) would be better suited for this project.

For the CD to work you need to get an API token from pypi and set it a secret called pypi_password (see)

After that, you can simply run

$ bump2version major|minor|patch
$ git push --follow-tags

And the release is on its way ^^

E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
for python runtime dependencies and used github-actions
quoting the addopts made them not being recognized properly
@s-weigand
Copy link
Contributor Author

Here is a demo how the checks in a PR would look like with GH actions.

And here is a demo of what the formatting could look like with yapf.
yapf can also be configured to ignore code blocks e.g. if you have a table formatted list.

@dr-prodigy
Copy link
Collaborator

Hey there @s-weigand great job! Sorry for being so late in answering: no time available in the recent past.
I've just started over with the new beta 0.10.5: I'll be analyzing your work in these days, and then let you know.
Thank you very much, KUTGW! 👍

@dr-prodigy
Copy link
Collaborator

dr-prodigy commented Feb 21, 2021

Hi @s-weigand ,
at last!
I could merge your brilliant work in the current beta status.. it required some fine tuning / fixes in order to run properly in fact, sometimes due to my (quite long, my bad!) last updates backlog.

At the moment, I've pushed it to "unstable" branch in order to complete all the final tests. Then if everything fine we'll move to beta, and then master, when a new version is released.

Would you please take a look and confirm everything is fine?

And, again, thank you very much for your outstanding work! 👍

@s-weigand
Copy link
Contributor Author

Sorry for the massive PR, I recently also had to review a PR of this size and it gets quite unhandy. 😓
Looking back I should better have split it up in at least a pytest and 'rest' PR.

Anyway glad you like the changes 😄

@s-weigand
Copy link
Contributor Author

I just ran the test suites

  • beta 'Ran 587 tests'
  • unstable 'collected 573 items'
    So it looks like those did diverge and IMHO the easiest way (already did a rebase once) to fix this is redoing the splitting up of test.py.

What do you think about me splitting this monster of a PR up to some smaller ones that are easier to review? 😄

Maybe leaving out the formatter for now, so you can have a look if yapf would be a better choice.
Due to the nature of this project, it contains a lot of data structures, where IMHO black hurt the readability with its opinionated formatting.

@s-weigand s-weigand marked this pull request as draft February 21, 2021 11:47
@s-weigand
Copy link
Contributor Author

I made a separate PR for changing the tests to pytest #438 , with that the amount of tests is in sync 😄

This was referenced Feb 21, 2021
@dr-prodigy
Copy link
Collaborator

Hey there @s-weigand , I think you caught me wrong .. I've already merged all of this PR on branch "unstable" (which you first should pull from), so no need to re-do it in parts.. sorry for your additional work! :-/

At the moment I'm in the phase of integrating the old test structure with your new one, in order to remove test duplications and temporarily keep travis aligned (still to be completely removed as a next task).

This given, there's still sth failing in the github actions build.. I'm going to investigate that too, but if you could help me in it, I would really appreciate :-)

If I come to a fix I'll update you here, thx! 👍

@s-weigand
Copy link
Contributor Author

@dr-prodigy in https://github.com/dr-prodigy/python-holidays/pull/404#issuecomment-782844414 I was talking about the "unstable".
I pulled your versions of beta + unstable and ran pytest. The problem is that already the amount of tests mismatched (587 for beta and 573 for unstable).

The problem with this PR is that it touched too many files (tests + holiday source) and once some merges get in between and it is out of sync it is virtually impossible (at last very exhausting) to sync all the diffs again w/o regression.
This is why I opened #438, so to say a start with a clean slate (HEAD of beta) for the pytest transition since the worst-case scenario would be that a test class is in the wrong file.
IMHO splitting up the PR and taking one step at a time would be the cleanest approach since I just copy-pasted test classes from tests.py to a corresponding file and additions in the holidays package can't get lost since those files weren't touched anyway.

I feel sorry for us both, having done extra work, just cause past me got a bit overexcited and wanted to do too many things in one PR 😓

@s-weigand
Copy link
Contributor Author

About the error with github action build, this is due to a new release of coveralls which needs an extra flag to play nicely with gh-actions see dr-prodigy@8946f01

@s-weigand
Copy link
Contributor Author

Oh, and btw if you want to make small changes on top of any of PR's (e.g. README change in 1774c25 for #438) my PR's are set to allow commits to it.

@dr-prodigy
Copy link
Collaborator

dr-prodigy commented Feb 22, 2021

Hi @s-weigand at the moment in "unstable" you'll find a quite valid situation, successfully building, and with successfully merged tests, ie:

  • removed all original tests from tests.py
  • imported your test classes from there
  • reviewed test_holiday_base.py adding all code for holiday base testing, previously available in tests.py

What is not clear to me is why the coverage is not 100% anymore:

---------- coverage: platform darwin, python 3.8.0-final-0 -----------
Name                                          Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------------
holidays/constants.py                             3      0      0      0   100%
holidays/countries/__init__.py                   70      0      0      0   100%
holidays/countries/angola.py                     49      0     24      2    97%
holidays/countries/argentina.py                  74      0     30      0   100%
holidays/countries/aruba.py                      38      0     12      0   100%
holidays/countries/australia.py                 162      0    120      1    99%
holidays/countries/austria.py                    32      0      4      0   100%
holidays/countries/bangladesh.py                 19      0      0      0   100%
holidays/countries/belarus.py                    27      0      4      0   100%
holidays/countries/belgium.py                    27      0      0      0   100%
holidays/countries/brazil.py                    105      0     54      0   100%
holidays/countries/bulgaria.py                   30      0      2      0   100%
holidays/countries/burundi.py                    55      0     18      0   100%
holidays/countries/canada.py                    152      0    114      4    98%
holidays/countries/chile.py                      73      0     30      2    98%
holidays/countries/colombia.py                   73      0     26      0   100%
holidays/countries/croatia.py                    34      0      8      0   100%
holidays/countries/czechia.py                    44      0     20      7    89%
holidays/countries/denmark.py                    26      0      0      0   100%
holidays/countries/djibouti.py                   39      0     10      0   100%
holidays/countries/dominican_republic.py         39      0      6      1    98%
holidays/countries/egypt.py                      50      0     18      3    96%
holidays/countries/estonia.py                    27      0      0      0   100%
holidays/countries/european_central_bank.py      21      0      0      0   100%
holidays/countries/finland.py                    30      0      0      0   100%
holidays/countries/france.py                     66      0     44      6    95%
holidays/countries/germany.py                    57      0     32      0   100%
holidays/countries/greece.py                     26      0      0      0   100%
holidays/countries/honduras.py                   66      0     34     16    84%
holidays/countries/hongkong.py                  184      0     74      1    99%
holidays/countries/hungary.py                    57      0     34      0   100%
holidays/countries/iceland.py                    30      0      0      0   100%
holidays/countries/india.py                      63      0     38      2    98%
holidays/countries/ireland.py                    49      5     24      7    84%
holidays/countries/israel.py                     78      0     16      1    99%
holidays/countries/italy.py                     109      0     84      0   100%
holidays/countries/japan.py                     124      0     92      5    98%
holidays/countries/kenya.py                      27      0      4      0   100%
holidays/countries/korea.py                     116      0     34      2    99%
holidays/countries/latvia.py                     32      0      8      4    90%
holidays/countries/lithuania.py                  33      0      8      3    93%
holidays/countries/luxembourg.py                 26      0      2      1    96%
holidays/countries/malawi.py                     36      0     10      2    96%
holidays/countries/mexico.py                     61      0     36      0   100%
holidays/countries/morocco.py                    49      0     20      2    97%
holidays/countries/mozambique.py                 36     23     10      0    28%
holidays/countries/netherlands.py                41      0     14      0   100%
holidays/countries/new_zealand.py               146      0     86      0   100%
holidays/countries/nicaragua.py                  30      0      6      3    92%
holidays/countries/nigeria.py                    19      0      0      0   100%
holidays/countries/norway.py                     52      0      6      0   100%
holidays/countries/paraguay.py                   59      0     22      0   100%
holidays/countries/peru.py                       35      0      0      0   100%
holidays/countries/poland.py                     37      0      8      4    91%
holidays/countries/portugal.py                   42      0      2      1    98%
holidays/countries/romania.py                    30      0      8      0   100%
holidays/countries/russia.py                     28      0      2      0   100%
holidays/countries/saudi_arabia.py               62     48     32      0    15%
holidays/countries/serbia.py                     40      0      8      0   100%
holidays/countries/singapore.py                 115      0     46      1    99%
holidays/countries/slovakia.py                   39      0      6      3    93%
holidays/countries/slovenia.py                   32      0      6      1    97%
holidays/countries/south_africa.py              113      0     72      1    99%
holidays/countries/spain.py                      86      0     58      0   100%
holidays/countries/sweden.py                     65      0     14      0   100%
holidays/countries/switzerland.py                64      0     40      1    99%
holidays/countries/turkey.py                     33      0      6      1    97%
holidays/countries/ukraine.py                    65      0     44      2    98%
holidays/countries/united_arab_emirates.py       81      0     38      1    99%
holidays/countries/united_kingdom.py            174      3    114      6    97%
holidays/countries/united_states.py             336      0    288      1    99%
holidays/countries/vietnam.py                    49      0     12      2    97%
holidays/holiday_base.py                        163      0     88      0   100%
holidays/utils.py                                26      6     14      0    70%
-------------------------------------------------------------------------------
TOTAL                                          4686     85   2144    100    97%

Did you modify test classes code prior to moving to the new test class files?
Would you please double-check?
Thx

@dr-prodigy
Copy link
Collaborator

PS: before rolling back everything and re-applying fixes again, one by one (which I'm not sure I'll be able to process soon :-/ ), I'd like you to check this last version.. theoretically restoring the right tests should bring us back to 100% coverage.

PPS: about formatting etc., I understand your point and agree, but at the moment I think we can live with that. But thank you for pointing it out.

Pls let me know, thank you very much

@s-weigand
Copy link
Contributor Author

I didn't modify the test classes, but only copied them to different files.
But this PR is based on a5eb566 and since then about 26 commits were added to the current beta ff1a39c, which is why this PR has outdated tests and holiday code.
That said I can confirm the 97% on unstable d4fa424, but on the branch for #438 8ca6ee0 I get 99%:

pytest results for #438, branch = true
----------- coverage: platform win32, python 3.7.6-final-0 -----------
Name                                          Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------------
__init__.py                                       0      0      0      0   100%
holidays\constants.py                             3      0      0      0   100%
holidays\countries\__init__.py                   70      0      0      0   100%
holidays\countries\angola.py                     49      0     24      2    97%
holidays\countries\argentina.py                  74      0     30      0   100%
holidays\countries\aruba.py                      38      0     12      0   100%
holidays\countries\australia.py                 162      0    120      1    99%
holidays\countries\austria.py                    32      0      4      0   100%
holidays\countries\bangladesh.py                 19      0      0      0   100%
holidays\countries\belarus.py                    27      0      4      0   100%
holidays\countries\belgium.py                    27      0      0      0   100%
holidays\countries\brazil.py                    105      0     54      0   100%
holidays\countries\bulgaria.py                   30      0      2      0   100%
holidays\countries\burundi.py                    55      0     18      0   100%
holidays\countries\canada.py                    152      0    114      4    98%
holidays\countries\chile.py                      73      0     30      2    98%
holidays\countries\colombia.py                   73      0     26      0   100%
holidays\countries\croatia.py                    34      0      8      0   100%
holidays\countries\czechia.py                    44      0     20      7    89%
holidays\countries\denmark.py                    26      0      0      0   100%
holidays\countries\djibouti.py                   39      0     10      0   100%
holidays\countries\dominican_republic.py         39      0      6      1    98%
holidays\countries\egypt.py                      50      0     18      3    96%
holidays\countries\estonia.py                    27      0      0      0   100%
holidays\countries\european_central_bank.py      21      0      0      0   100%
holidays\countries\finland.py                    30      0      0      0   100%
holidays\countries\france.py                     66      0     44      6    95%
holidays\countries\germany.py                    57      0     32      0   100%
holidays\countries\greece.py                     26      0      0      0   100%
holidays\countries\honduras.py                   66      0     34     16    84%
holidays\countries\hongkong.py                  184      0     74      1    99%
holidays\countries\hungary.py                    57      0     34      0   100%
holidays\countries\iceland.py                    30      0      0      0   100%
holidays\countries\india.py                      63      0     38      2    98%
holidays\countries\ireland.py                    49      0     24      1    99%
holidays\countries\israel.py                     78      0     16      1    99%
holidays\countries\italy.py                     109      0     84      0   100%
holidays\countries\japan.py                     124      0     92      5    98%
holidays\countries\kenya.py                      27      0      4      0   100%
holidays\countries\korea.py                     116      0     34      2    99%
holidays\countries\latvia.py                     32      0      8      4    90%
holidays\countries\lithuania.py                  33      0      8      3    93%
holidays\countries\luxembourg.py                 26      0      2      1    96%
holidays\countries\malawi.py                     36      0     10      2    96%
holidays\countries\mexico.py                     61      0     36      0   100%
holidays\countries\morocco.py                    49      0     20      2    97%
holidays\countries\mozambique.py                 36      0     10      2    96%
holidays\countries\netherlands.py                41      0     14      0   100%
holidays\countries\new_zealand.py               146      0     86      0   100%
holidays\countries\nicaragua.py                  30      0      6      3    92%
holidays\countries\nigeria.py                    19      0      0      0   100%
holidays\countries\norway.py                     52      0      6      0   100%
holidays\countries\paraguay.py                   59      0     22      0   100%
holidays\countries\peru.py                       35      0      0      0   100%
holidays\countries\poland.py                     37      0      8      4    91%
holidays\countries\portugal.py                   42      0      2      1    98%
holidays\countries\romania.py                    30      0      8      0   100%
holidays\countries\russia.py                     28      0      2      0   100%
holidays\countries\saudi_arabia.py               62      0     32      0   100%
holidays\countries\serbia.py                     40      0      8      0   100%
holidays\countries\singapore.py                 115      0     46      1    99%
holidays\countries\slovakia.py                   39      0      6      3    93%
holidays\countries\slovenia.py                   32      0      6      1    97%
holidays\countries\south_africa.py              113      0     72      1    99%
holidays\countries\spain.py                      86      0     58      0   100%
holidays\countries\sweden.py                     65      0     14      0   100%
holidays\countries\switzerland.py                64      0     40      1    99%
holidays\countries\turkey.py                     33      0      6      1    97%
holidays\countries\ukraine.py                    65      0     44      2    98%
holidays\countries\united_arab_emirates.py       81      0     38      1    99%
holidays\countries\united_kingdom.py            144      0     90      3    99%
holidays\countries\united_states.py             336      0    288      1    99%
holidays\countries\vietnam.py                    49      0     12      2    97%
holidays\holiday_base.py                        163      0     88      0   100%
holidays\utils.py                                19      0      8      0   100%
-------------------------------------------------------------------------------
TOTAL                                          4649      0   2114     93    99%
Coverage XML written to file coverage.xml


========================================= 587 passed in 58.79s =========================================
pytest results for `beta` branch = true
----------- coverage: platform win32, python 3.7.6-final-0 -----------
Name                                          Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------------
__init__.py                                       0      0      0      0   100%
holidays\constants.py                             3      0      0      0   100%
holidays\countries\__init__.py                   70      0      0      0   100%
holidays\countries\angola.py                     49      0     24      2    97%
holidays\countries\argentina.py                  74      0     30      0   100%
holidays\countries\aruba.py                      38      0     12      0   100%
holidays\countries\australia.py                 162      0    120      1    99%
holidays\countries\austria.py                    32      0      4      0   100%
holidays\countries\bangladesh.py                 19      0      0      0   100%
holidays\countries\belarus.py                    27      0      4      0   100%
holidays\countries\belgium.py                    27      0      0      0   100%
holidays\countries\brazil.py                    105      0     54      0   100%
holidays\countries\bulgaria.py                   30      0      2      0   100%
holidays\countries\burundi.py                    55      0     18      0   100%
holidays\countries\canada.py                    152      0    114      4    98%
holidays\countries\chile.py                      73      0     30      2    98%
holidays\countries\colombia.py                   73      0     26      0   100%
holidays\countries\croatia.py                    34      0      8      0   100%
holidays\countries\czechia.py                    44      0     20      7    89%
holidays\countries\denmark.py                    26      0      0      0   100%
holidays\countries\djibouti.py                   39      0     10      0   100%
holidays\countries\dominican_republic.py         39      0      6      1    98%
holidays\countries\egypt.py                      50      0     18      3    96%
holidays\countries\estonia.py                    27      0      0      0   100%
holidays\countries\european_central_bank.py      21      0      0      0   100%
holidays\countries\finland.py                    30      0      0      0   100%
holidays\countries\france.py                     66      0     44      6    95%
holidays\countries\germany.py                    57      0     32      0   100%
holidays\countries\greece.py                     26      0      0      0   100%
holidays\countries\honduras.py                   66      0     34     16    84%
holidays\countries\hongkong.py                  184      0     74      1    99%
holidays\countries\hungary.py                    57      0     34      0   100%
holidays\countries\iceland.py                    30      0      0      0   100%
holidays\countries\india.py                      63      0     38      2    98%
holidays\countries\ireland.py                    49      0     24      1    99%
holidays\countries\israel.py                     78      0     16      1    99%
holidays\countries\italy.py                     109      0     84      0   100%
holidays\countries\japan.py                     124      0     92      5    98%
holidays\countries\kenya.py                      27      0      4      0   100%
holidays\countries\korea.py                     116      0     34      2    99%
holidays\countries\latvia.py                     32      0      8      4    90%
holidays\countries\lithuania.py                  33      0      8      3    93%
holidays\countries\luxembourg.py                 26      0      2      1    96%
holidays\countries\malawi.py                     36      0     10      2    96%
holidays\countries\mexico.py                     61      0     36      0   100%
holidays\countries\morocco.py                    49      0     20      2    97%
holidays\countries\mozambique.py                 36      0     10      2    96%
holidays\countries\netherlands.py                41      0     14      0   100%
holidays\countries\new_zealand.py               146      0     86      0   100%
holidays\countries\nicaragua.py                  30      0      6      3    92%
holidays\countries\nigeria.py                    19      0      0      0   100%
holidays\countries\norway.py                     52      0      6      0   100%
holidays\countries\paraguay.py                   59      0     22      0   100%
holidays\countries\peru.py                       35      0      0      0   100%
holidays\countries\poland.py                     37      0      8      4    91%
holidays\countries\portugal.py                   42      0      2      1    98%
holidays\countries\romania.py                    30      0      8      0   100%
holidays\countries\russia.py                     28      0      2      0   100%
holidays\countries\saudi_arabia.py               62      0     32      0   100%
holidays\countries\serbia.py                     40      0      8      0   100%
holidays\countries\singapore.py                 115      0     46      1    99%
holidays\countries\slovakia.py                   39      0      6      3    93%
holidays\countries\slovenia.py                   32      0      6      1    97%
holidays\countries\south_africa.py              113      0     72      1    99%
holidays\countries\spain.py                      86      0     58      0   100%
holidays\countries\sweden.py                     65      0     14      0   100%
holidays\countries\switzerland.py                64      0     40      1    99%
holidays\countries\turkey.py                     33      0      6      1    97%
holidays\countries\ukraine.py                    65      0     44      2    98%
holidays\countries\united_arab_emirates.py       81      0     38      1    99%
holidays\countries\united_kingdom.py            144      0     90      3    99%
holidays\countries\united_states.py             336      0    288      1    99%
holidays\countries\vietnam.py                    49      0     12      2    97%
holidays\holiday_base.py                        163      0     88      0   100%
holidays\utils.py                                19      0      8      0   100%
tests.py                                       4841      0    968     23    99%
-------------------------------------------------------------------------------
TOTAL                                          9490      0   3082    116    99%
Coverage XML written to file coverage.xml


==================================== 587 passed in 63.91s (0:01:03) ====================================

For running pytest on beta ff1a39c I used the command pytest tests.py and the config from pyproject.toml, so the results are comparable.

The reason for the coverage on #438 and beta being 99% instead of 100% is due to the coverage settings in pyproject.toml which are set to consider branch coverage (if each case of an if-elif-else case was hit):

[tool.coverage.run]
branch = true

I don't know about you, but I prefer an honest 99% (which is already pretty awesome and rare) over a shiny 100% which doesn't tell the whole truth.

pytest results for `beta`, branch = false
----------- coverage: platform win32, python 3.7.6-final-0 -----------
Name                                          Stmts   Miss  Cover
-----------------------------------------------------------------
__init__.py                                       0      0   100%
holidays\constants.py                             3      0   100%
holidays\countries\__init__.py                   70      0   100%
holidays\countries\angola.py                     49      0   100%
holidays\countries\argentina.py                  74      0   100%
holidays\countries\aruba.py                      38      0   100%
holidays\countries\australia.py                 162      0   100%
holidays\countries\austria.py                    32      0   100%
holidays\countries\bangladesh.py                 19      0   100%
holidays\countries\belarus.py                    27      0   100%
holidays\countries\belgium.py                    27      0   100%
holidays\countries\brazil.py                    105      0   100%
holidays\countries\bulgaria.py                   30      0   100%
holidays\countries\burundi.py                    55      0   100%
holidays\countries\canada.py                    152      0   100%
holidays\countries\chile.py                      73      0   100%
holidays\countries\colombia.py                   73      0   100%
holidays\countries\croatia.py                    34      0   100%
holidays\countries\czechia.py                    44      0   100%
holidays\countries\denmark.py                    26      0   100%
holidays\countries\djibouti.py                   39      0   100%
holidays\countries\dominican_republic.py         39      0   100%
holidays\countries\egypt.py                      50      0   100%
holidays\countries\estonia.py                    27      0   100%
holidays\countries\european_central_bank.py      21      0   100%
holidays\countries\finland.py                    30      0   100%
holidays\countries\france.py                     66      0   100%
holidays\countries\germany.py                    57      0   100%
holidays\countries\greece.py                     26      0   100%
holidays\countries\honduras.py                   66      0   100%
holidays\countries\hongkong.py                  184      0   100%
holidays\countries\hungary.py                    57      0   100%
holidays\countries\iceland.py                    30      0   100%
holidays\countries\india.py                      63      0   100%
holidays\countries\ireland.py                    49      0   100%
holidays\countries\israel.py                     78      0   100%
holidays\countries\italy.py                     109      0   100%
holidays\countries\japan.py                     124      0   100%
holidays\countries\kenya.py                      27      0   100%
holidays\countries\korea.py                     116      0   100%
holidays\countries\latvia.py                     32      0   100%
holidays\countries\lithuania.py                  33      0   100%
holidays\countries\luxembourg.py                 26      0   100%
holidays\countries\malawi.py                     36      0   100%
holidays\countries\mexico.py                     61      0   100%
holidays\countries\morocco.py                    49      0   100%
holidays\countries\mozambique.py                 36      0   100%
holidays\countries\netherlands.py                41      0   100%
holidays\countries\new_zealand.py               146      0   100%
holidays\countries\nicaragua.py                  30      0   100%
holidays\countries\nigeria.py                    19      0   100%
holidays\countries\norway.py                     52      0   100%
holidays\countries\paraguay.py                   59      0   100%
holidays\countries\peru.py                       35      0   100%
holidays\countries\poland.py                     37      0   100%
holidays\countries\portugal.py                   42      0   100%
holidays\countries\romania.py                    30      0   100%
holidays\countries\russia.py                     28      0   100%
holidays\countries\saudi_arabia.py               62      0   100%
holidays\countries\serbia.py                     40      0   100%
holidays\countries\singapore.py                 115      0   100%
holidays\countries\slovakia.py                   39      0   100%
holidays\countries\slovenia.py                   32      0   100%
holidays\countries\south_africa.py              113      0   100%
holidays\countries\spain.py                      86      0   100%
holidays\countries\sweden.py                     65      0   100%
holidays\countries\switzerland.py                64      0   100%
holidays\countries\turkey.py                     33      0   100%
holidays\countries\ukraine.py                    65      0   100%
holidays\countries\united_arab_emirates.py       81      0   100%
holidays\countries\united_kingdom.py            144      0   100%
holidays\countries\united_states.py             336      0   100%
holidays\countries\vietnam.py                    49      0   100%
holidays\holiday_base.py                        163      0   100%
holidays\utils.py                                19      0   100%
-----------------------------------------------------------------
TOTAL                                          4649      0   100%
Coverage XML written to file coverage.xml


========================================= 587 passed in 57.93s =========================================
pytest results for #438, branch = false
----------- coverage: platform win32, python 3.7.6-final-0 -----------
Name                                          Stmts   Miss  Cover
-----------------------------------------------------------------
__init__.py                                       0      0   100%
holidays\constants.py                             3      0   100%
holidays\countries\__init__.py                   70      0   100%
holidays\countries\angola.py                     49      0   100%
holidays\countries\argentina.py                  74      0   100%
holidays\countries\aruba.py                      38      0   100%
holidays\countries\australia.py                 162      0   100%
holidays\countries\austria.py                    32      0   100%
holidays\countries\bangladesh.py                 19      0   100%
holidays\countries\belarus.py                    27      0   100%
holidays\countries\belgium.py                    27      0   100%
holidays\countries\brazil.py                    105      0   100%
holidays\countries\bulgaria.py                   30      0   100%
holidays\countries\burundi.py                    55      0   100%
holidays\countries\canada.py                    152      0   100%
holidays\countries\chile.py                      73      0   100%
holidays\countries\colombia.py                   73      0   100%
holidays\countries\croatia.py                    34      0   100%
holidays\countries\czechia.py                    44      0   100%
holidays\countries\denmark.py                    26      0   100%
holidays\countries\djibouti.py                   39      0   100%
holidays\countries\dominican_republic.py         39      0   100%
holidays\countries\egypt.py                      50      0   100%
holidays\countries\estonia.py                    27      0   100%
holidays\countries\european_central_bank.py      21      0   100%
holidays\countries\finland.py                    30      0   100%
holidays\countries\france.py                     66      0   100%
holidays\countries\germany.py                    57      0   100%
holidays\countries\greece.py                     26      0   100%
holidays\countries\honduras.py                   66      0   100%
holidays\countries\hongkong.py                  184      0   100%
holidays\countries\hungary.py                    57      0   100%
holidays\countries\iceland.py                    30      0   100%
holidays\countries\india.py                      63      0   100%
holidays\countries\ireland.py                    49      0   100%
holidays\countries\israel.py                     78      0   100%
holidays\countries\italy.py                     109      0   100%
holidays\countries\japan.py                     124      0   100%
holidays\countries\kenya.py                      27      0   100%
holidays\countries\korea.py                     116      0   100%
holidays\countries\latvia.py                     32      0   100%
holidays\countries\lithuania.py                  33      0   100%
holidays\countries\luxembourg.py                 26      0   100%
holidays\countries\malawi.py                     36      0   100%
holidays\countries\mexico.py                     61      0   100%
holidays\countries\morocco.py                    49      0   100%
holidays\countries\mozambique.py                 36      0   100%
holidays\countries\netherlands.py                41      0   100%
holidays\countries\new_zealand.py               146      0   100%
holidays\countries\nicaragua.py                  30      0   100%
holidays\countries\nigeria.py                    19      0   100%
holidays\countries\norway.py                     52      0   100%
holidays\countries\paraguay.py                   59      0   100%
holidays\countries\peru.py                       35      0   100%
holidays\countries\poland.py                     37      0   100%
holidays\countries\portugal.py                   42      0   100%
holidays\countries\romania.py                    30      0   100%
holidays\countries\russia.py                     28      0   100%
holidays\countries\saudi_arabia.py               62      0   100%
holidays\countries\serbia.py                     40      0   100%
holidays\countries\singapore.py                 115      0   100%
holidays\countries\slovakia.py                   39      0   100%
holidays\countries\slovenia.py                   32      0   100%
holidays\countries\south_africa.py              113      0   100%
holidays\countries\spain.py                      86      0   100%
holidays\countries\sweden.py                     65      0   100%
holidays\countries\switzerland.py                64      0   100%
holidays\countries\turkey.py                     33      0   100%
holidays\countries\ukraine.py                    65      0   100%
holidays\countries\united_arab_emirates.py       81      0   100%
holidays\countries\united_kingdom.py            144      0   100%
holidays\countries\united_states.py             336      0   100%
holidays\countries\vietnam.py                    49      0   100%
holidays\holiday_base.py                        163      0   100%
holidays\utils.py                                19      0   100%
-----------------------------------------------------------------
TOTAL                                          4649      0   100%
Coverage XML written to file coverage.xml


========================================= 587 passed in 57.93s =========================================

On unstable I only get 98% with branch coverage deactivated

pytest results for `unstable`, branch = false
----------- coverage: platform win32, python 3.7.6-final-0 -----------
Name                                          Stmts   Miss  Cover
-----------------------------------------------------------------
holidays\constants.py                             3      0   100%
holidays\countries\__init__.py                   70      0   100%
holidays\countries\angola.py                     49      0   100%
holidays\countries\argentina.py                  74      0   100%
holidays\countries\aruba.py                      38      0   100%
holidays\countries\australia.py                 162      0   100%
holidays\countries\austria.py                    32      0   100%
holidays\countries\bangladesh.py                 19      0   100%
holidays\countries\belarus.py                    27      0   100%
holidays\countries\belgium.py                    27      0   100%
holidays\countries\brazil.py                    105      0   100%
holidays\countries\bulgaria.py                   30      0   100%
holidays\countries\burundi.py                    55      0   100%
holidays\countries\canada.py                    152      0   100%
holidays\countries\chile.py                      73      0   100%
holidays\countries\colombia.py                   73      0   100%
holidays\countries\croatia.py                    34      0   100%
holidays\countries\czechia.py                    44      0   100%
holidays\countries\denmark.py                    26      0   100%
holidays\countries\djibouti.py                   39      0   100%
holidays\countries\dominican_republic.py         39      0   100%
holidays\countries\egypt.py                      50      0   100%
holidays\countries\estonia.py                    27      0   100%
holidays\countries\european_central_bank.py      21      0   100%
holidays\countries\finland.py                    30      0   100%
holidays\countries\france.py                     66      0   100%
holidays\countries\germany.py                    57      0   100%
holidays\countries\greece.py                     26      0   100%
holidays\countries\honduras.py                   66      0   100%
holidays\countries\hongkong.py                  184      0   100%
holidays\countries\hungary.py                    57      0   100%
holidays\countries\iceland.py                    30      0   100%
holidays\countries\india.py                      63      0   100%
holidays\countries\ireland.py                    49      5    90%
holidays\countries\israel.py                     78      0   100%
holidays\countries\italy.py                     109      0   100%
holidays\countries\japan.py                     124      0   100%
holidays\countries\kenya.py                      27      0   100%
holidays\countries\korea.py                     116      0   100%
holidays\countries\latvia.py                     32      0   100%
holidays\countries\lithuania.py                  33      0   100%
holidays\countries\luxembourg.py                 26      0   100%
holidays\countries\malawi.py                     36      0   100%
holidays\countries\mexico.py                     61      0   100%
holidays\countries\morocco.py                    49      0   100%
holidays\countries\mozambique.py                 36     23    36%
holidays\countries\netherlands.py                41      0   100%
holidays\countries\new_zealand.py               146      0   100%
holidays\countries\nicaragua.py                  30      0   100%
holidays\countries\nigeria.py                    19      0   100%
holidays\countries\norway.py                     52      0   100%
holidays\countries\paraguay.py                   59      0   100%
holidays\countries\peru.py                       35      0   100%
holidays\countries\poland.py                     37      0   100%
holidays\countries\portugal.py                   42      0   100%
holidays\countries\romania.py                    30      0   100%
holidays\countries\russia.py                     28      0   100%
holidays\countries\saudi_arabia.py               62     48    23%
holidays\countries\serbia.py                     40      0   100%
holidays\countries\singapore.py                 115      0   100%
holidays\countries\slovakia.py                   39      0   100%
holidays\countries\slovenia.py                   32      0   100%
holidays\countries\south_africa.py              113      0   100%
holidays\countries\spain.py                      86      0   100%
holidays\countries\sweden.py                     65      0   100%
holidays\countries\switzerland.py                64      0   100%
holidays\countries\turkey.py                     33      0   100%
holidays\countries\ukraine.py                    65      0   100%
holidays\countries\united_arab_emirates.py       81      0   100%
holidays\countries\united_kingdom.py            174      3    98%
holidays\countries\united_states.py             336      0   100%
holidays\countries\vietnam.py                    49      0   100%
holidays\holiday_base.py                        163      0   100%
holidays\utils.py                                26      6    77%
-----------------------------------------------------------------
TOTAL                                          4686     85    98%
Coverage XML written to file coverage.xml


========================================= 574 passed in 58.68s =========================================

So in the best case, there are only some tests missing on unstable.

Some sidenotes on the commits on unstable:

  • 3bdfa24 __init__.py aren't needed for pytest to discover tests since it discoveres tests by a naming patter (test_ by default) or test class subclasses from other frameworks e.g. unittest or nosetest
  • 6f31bc4 TestFlake8 isn't needed anymore since pre-commit checks the files both locally (only staged files if run as hook) and on the CI (for all tracked files, same as locally running pre-commit run -a)

dr-prodigy added a commit that referenced this pull request Feb 22, 2021
@dr-prodigy
Copy link
Collaborator

dr-prodigy commented Feb 22, 2021

Hi @s-weigand thank you! Some more work done..

I didn't modify the test classes, but only copied them to different files.
But this PR is based on a5eb566 and since then about 26 commits were added to the current beta ff1a39c, which is why this PR has outdated tests and holiday code.
That said I can confirm the 97% on unstable d4fa424, but on the branch for #438 8ca6ee0 I get 99%:

pytest results for #438, branch = true
pytest results for beta branch = true
For running pytest on beta ff1a39c I used the command pytest tests.py and the config from pyproject.toml, so the results are comparable.

Okay all clear.. having put a lot of effort in the last days to sort out everything, I finally decided to preserve what done, to re-apply the test changes still missing, and configured pytest to crawl all tests starting from tests.py , as it used to be on coverage.
This way, I have one only version of all the tests procedure, and Coveralls remains consistent with GH actions while I / we complete the code before merging in beta (but I would say we are pretty there now).

The reason for the coverage on #438 and beta being 99% instead of 100% is due to the coverage settings in pyproject.toml which are set to consider branch coverage (if each case of an if-elif-else case was hit):

[tool.coverage.run]
branch = true

I don't know about you, but I prefer an honest 99% (which is already pretty awesome and rare) over a shiny 100% which doesn't tell the whole truth.

Although I'm not very happy to lose my 100% badge :-) (btw, no cheating, it only went unnoticed in fact.. due to coverage behaving differently) it makes sense and I agree.. we'll see if we can get back to a "real" 100% in a future.

pytest results for beta, branch = false
pytest results for #438, branch = false
On unstable I only get 98% with branch coverage deactivated

pytest results for unstable, branch = false
So in the best case, there are only some tests missing on unstable.

Some sidenotes on the commits on unstable:

  • 3bdfa24 __init__.py aren't needed for pytest to discover tests since it discoveres tests by a naming patter (test_ by default) or test class subclasses from other frameworks e.g. unittest or nosetest

I was aware of this, but that's instead required to define tests.py as the entry point for the tests (which is the case now, in order to keep consistency with the past).. no harm and more standard anyway :-)

  • 6f31bc4 TestFlake8 isn't needed anymore since pre-commit checks the files both locally (only staged files if run as hook) and on the CI (for all tracked files, same as locally running pre-commit run -a)

Although, again, this was made to keep coverage sync while finalizing, it is in fact true, so I now decided to drop it. Thx!

As said before, we should theoretically be ready to merge to beta now.. would you please put a last eye on the last commits and confirm there's nothing breaking or misleaded in my changes (everything seems fine, but that's my first experience with GH actions and pytest, so better 2 eyes more)?

Thank you again, KUTGW! 👍

@s-weigand
Copy link
Contributor Author

I opened #441 against unstable.
There was a change to holidays/countries/united_kingdom.py in dr-prodigy@afc4074 that appeared to have slipped through the cracks.

Also, I found that there is no test for holidays.utils.is_leap_year yet, which explains the lover coverage.

Besides this, all LGTM

dr-prodigy added a commit that referenced this pull request Feb 24, 2021
…443)

* added coverage.xml to .gitignore

* Basic rewrite of the travis tests, with github actions

* removed now obsolete travis config

* Added bump2version and its config

* moved flake8 config to setup.cfg

* Formatted requirements with comments

* Created test file for each country and moved corresponding tests there

* Added pre-commit and a basic config containing black+its config

* Added flake8 to pre-commit config and set flake8 to ignore E203

E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)

* Replaced flake8 tests with pre-commit ones, since flake8 is included

* Updated workflow to use tests folder, upload coverage to coveralls.io 

and some minor fixes

* Added pre-commit hook that adds encoding shebang to all files

py27 compat

* Added coverage config

* Added tox+pytest config and added .tox to .gitignore

* Added rst checking tools  to pre-commit and fixed rst issues

* Added workflow dispatch as an option to run the workflow

* Removed py35 as was done in https://github.com/dr-prodigy/python-holidays/pull/402

* Added dependabot config to receive automatics update PR's 

for python runtime dependencies and used github-actions

* Fixed pytest config

quoting the addopts made them not being recognized properly

* Replaced travis with github actions badge

* Changed contributing guide to reflect changed tooling

* Formatted setup.py again with black

* Auto-format, Israel to_jd_gregorianyear fix

* requirements_dev.txt review, Israel fixes, CHANGES+README.rst reviews

* precommit tasks run

* removed python 2.7 checks from github CI/CD scripts

* flake8 config fixes

* travis build toml dependency fix

* removed duplicated flake8 tests from coverage config

* old tests.py using new test classes

* tests tree reviewed

* tests cleanup (warn: coverage report -m needs fixing)

* Fixed usage of coveralls with gh-action after 3.0.0 release

* more recent tests re-applied - #404

* copyright 2021

* test tree refactoring, pytest running thru tests.py

* Flake8 test removed, pyproject.toml cleanup

* Added .gitaddtributes file to ensure consistent '\n' line ending

for future commits

* Added pre-commit hook to enforce '\n' lineending and applied it on all

files

* Copied files from 'holiday' and 'tests' over and ran 'pre-commit run -a'

This should catch all changes on beta which were missing.
I left changes which were added in the cleanup of 'unstable' e.g. 'holidays.utils.is_leap_year'

'test/countries/test_saudiarabia.py' was renamed to 'test/countries/test_saudi_arabia.py' to be consistent with 'holidays/countries/saudi_arabia.py'

* is_leap_year removal

Co-authored-by: s-weigand <s.weigand.phy@gmail.com>
dr-prodigy added a commit that referenced this pull request Feb 24, 2021
(massive thx+kudos to s-weigand)

* added coverage.xml to .gitignore

* Basic rewrite of the travis tests, with github actions

* removed now obsolete travis config

* Added bump2version and its config

* moved flake8 config to setup.cfg

* Formatted requirements with comments

* Created test file for each country and moved corresponding tests there

* Added pre-commit and a basic config containing black+its config

* Added flake8 to pre-commit config and set flake8 to ignore E203

E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)

* Replaced flake8 tests with pre-commit ones, since flake8 is included

* Updated workflow to use tests folder, upload coverage to coveralls.io

and some minor fixes

* Added pre-commit hook that adds encoding shebang to all files

py27 compat

* Added coverage config

* Added tox+pytest config and added .tox to .gitignore

* Added rst checking tools  to pre-commit and fixed rst issues

* Added workflow dispatch as an option to run the workflow

* Removed py35 as was done in https://github.com/dr-prodigy/python-holidays/pull/402

* Added dependabot config to receive automatics update PR's

for python runtime dependencies and used github-actions

* Fixed pytest config

quoting the addopts made them not being recognized properly

* Replaced travis with github actions badge

* Changed contributing guide to reflect changed tooling

* Formatted setup.py again with black

* Auto-format, Israel to_jd_gregorianyear fix

* requirements_dev.txt review, Israel fixes, CHANGES+README.rst reviews

* precommit tasks run

* removed python 2.7 checks from github CI/CD scripts

* flake8 config fixes

* travis build toml dependency fix

* removed duplicated flake8 tests from coverage config

* old tests.py using new test classes

* tests tree reviewed

* tests cleanup (warn: coverage report -m needs fixing)

* Fixed usage of coveralls with gh-action after 3.0.0 release

* more recent tests re-applied - #404

* copyright 2021

* test tree refactoring, pytest running thru tests.py

* Flake8 test removed, pyproject.toml cleanup

* Added .gitaddtributes file to ensure consistent '\n' line ending

for future commits

* Added pre-commit hook to enforce '\n' lineending and applied it on all

files

* Copied files from 'holiday' and 'tests' over and ran 'pre-commit run -a'

This should catch all changes on beta which were missing.
I left changes which were added in the cleanup of 'unstable' e.g. 'holidays.utils.is_leap_year'

'test/countries/test_saudiarabia.py' was renamed to 'test/countries/test_saudi_arabia.py' to be consistent with 'holidays/countries/saudi_arabia.py'

* is_leap_year removal

Co-authored-by: s-weigand <s.weigand.phy@gmail.com>
@dr-prodigy
Copy link
Collaborator

Final version reviewed and released in beta.
Thx!

@dr-prodigy dr-prodigy closed this Feb 24, 2021
@dr-prodigy
Copy link
Collaborator

hi @s-weigand how are you doing?
I'm referring back to this because I'm currently in the phase of running bump2version for the first time to release the new v0.10.5.3, but it seems to lack some required information. In particular, here's the raised exception:

$ bump2version patch        
Traceback (most recent call last):
  File "/Users/mauri/Documents/git/venv_holidays38/bin/bump2version", line 8, in <module>
    sys.exit(main())
  File "/Users/mauri/Documents/git/venv_holidays38/lib/python3.8/site-packages/bumpversion/cli.py", line 124, in main
    _check_files_contain_version(files, current_version, context)
  File "/Users/mauri/Documents/git/venv_holidays38/lib/python3.8/site-packages/bumpversion/cli.py", line 618, in _check_files_contain_version
    f.should_contain_version(current_version, context)
  File "/Users/mauri/Documents/git/venv_holidays38/lib/python3.8/site-packages/bumpversion/utils.py", line 68, in should_contain_version
    raise VersionNotFoundException(
bumpversion.exceptions.VersionNotFoundException: Did not find 'version='0.10.5',' in file: 'setup.py'

Any ideas?
Thank you cheers 👍

@dr-prodigy dr-prodigy reopened this Mar 30, 2021
@s-weigand
Copy link
Contributor Author

@dr-prodigy Hey, sorry I guess that slipped my attention.
The way bump2version works when replacing versions is basically just an exact regex matching and replacing.
So it is looking for:

    version='0.10.5',

and only found

https://github.com/dr-prodigy/python-holidays/blob/246c729718907d58b6cbf9a7bab045b7354e006c/setup.py#L32

thus it crashes.

I just started a quick fix branch and found another problem. bump2version by default only supports versions in the PEP440 format, which has the pattern <major>.<minor>.<patch>{.devN, aN, bN, rcN, <no suffix>, .postN}, thus 0.10.5.3 isn't a valid version for it and it needs some extra configuration.

The problem with that extra configuration is that a line would end with = and bump2version would add a trailing whitespace (known issue), which in turn lead to a fight with the pre-commit hook that removes them and the commit fails.
That behavior is why I recently stopped using bump2version on new projects and instead use a setup.cfg only pattern and create a tag via the github release.

With a setup.cfg only pattern you also only have to manage the version in one place (__init__.py), since you can refer to __version__ as an attribute, in your case this would be

version = attr: holidays.__version__

@s-weigand s-weigand closed this Apr 2, 2021
@s-weigand s-weigand deleted the use-gh-action-ci-cd branch April 18, 2021 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants