Studentbrad/pytest

[studentbrad]

Moving the Tests Folder

Description

Pointed out by @itSeez, the tests folder was in the wrong location. We believed that CodeCov was incorrectly monitoring the coverage of the test files due to the tests folder being under the jobfunnel directory (a mistake). I moved the tests folder outside of the package (as it should be) and excluded it from the pip installation (as it should be).

As it turns out, this was not the core issue, but had to be done regardless. CodeCov intentionally monitors the converge of the tests folder and includes it in the CodeCov report. It is disputed in the community whether it should be excluded or not. https://blog.codecov.io/2019/10/25/should-i-include-test-files-in-code-coverage-calculations/

I chose to not exclude it. I don't have any reason for that, I just chose not to. Therefore, these changes are simply to restructure the tests directory.

Context of change

Please add options that are relevant and mark any boxes that apply.

• [ ] Software (software that runs on the PC)
• [ ] Library (library that runs on the PC)
• [ ] Tool (tool that assists coding development)
• [x] Other

Type of change

Please mark any boxes that apply.

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update

How Has This Been Tested?

This has been tested in Travis-CI. I have run several tests to restore functionality with the new directory structure.

Checklist:

Please mark any boxes that have been completed.

• [x] I have performed a self-review of my own code.
• [x] I have commented my code, particularly in hard-to-understand areas.
• [x] I have made corresponding changes to the documentation.
• [x] My changes generate no new warnings.
• [x] I have added tests that prove my fix is effective or that my feature works.
• [x] New and existing unit tests pass locally with my changes.
• [x] Any dependent changes have been merged and published in downstream modules.

[codecov-io]

Codecov Report

Merging #70 into master will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   49.62%   49.64%   +0.02%     
==========================================
  Files          18       18              
  Lines        1991     1994       +3     
==========================================
+ Hits          988      990       +2     
- Misses       1003     1004       +1     

Impacted Files	Coverage Δ
jobfunnel/__init__.py	100.00% <100.00%> (ø)
tests/conftest.py	96.55% <100.00%> (ø)
tests/test_countries.py	96.87% <100.00%> (ø)
tests/test_filters.py	100.00% <100.00%> (ø)
tests/test_parse.py	100.00% <100.00%> (ø)
tests/test_tools.py	100.00% <100.00%> (ø)
tests/test_validate.py	100.00% <100.00%> (ø)
jobfunnel/config/__init__.py	19.93% <0.00%> (-0.16%)	:arrow_down:

---

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 2691a42...9f64971. Read the comment docs.

[markkvdb]

Good observation. I never noticed that the tests directory was inside the src. It is indeed the right decision to separate them also because installing the package with pip should not include the tests.

For the inclusion of the tests directory for the code coverage. Personally, I think it's better to leave out the tests directory, since it does not reflect what we want to measure with the code coverage. We want to measure our code base which is inside the jobfunnel folder. The coverage of the tests folder is by definition close to 100%, so it doesn't really inform us about anything meaningful. The thing that it does do however is that the nominator and the denominator of the code coverage statistic gets 'distorted'. However, if you guys prefer keeping the tests in the test coverage that's fine by me.

[studentbrad]

Good point, I will be excluding it. It will definitely take a hit on the coverage (<40%) but I’m sure it will go back up.

[markkvdb]

Yes, the coverage will take a hit but I think it's only fair. Even more reasons to work hard to increase the test coverage! Cheers.

[studentbrad]

There is somewhat of a contradiction with CodeCov and the best practices in coding python packages. Usually we do not include the __init__.py file in tests because it shouldn't be included in the pip package. However, I learned that CodeCov will report 0% coverage if it is not present. This should not be a problem because I omitted tests from the pip installation here: https://github.com/studentbrad/JobFunnel/blob/7dc5b132107f2ce7d0061e93f8791f263645301b/setup.py#L33

I just thought I'd mention it.