Simplify and deduplicate test code

[mulhod]

Closes #635.

[pep8speaks]

Hello @mulhod! Thanks for updating this PR.

• In the file tests/test_classification.py:

Line 721:101: E501 line too long (102 > 100 characters)

• In the file tests/test_input.py:

Line 1148:101: E501 line too long (102 > 100 characters)

Comment last updated at 2020-11-13 17:39:06 UTC

[hlepp]

Why remove import?

[mulhod]

It was only being used for os.unlink, which isn't being used anymore. Now pathlib.Path.unlink is being used (but it's actually used indirectly -- it's in a helper method in tests.utils that coerces the argument to a Path so that we don't have to care about whether the path is a str or a Path).

[codecov]

Codecov Report

Merging #642 (2daaa24) into main (3dafc5f) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #642   +/-   ##
=======================================
  Coverage   95.11%   95.11%           
=======================================
  Files          27       27           
  Lines        3089     3089           
=======================================
  Hits         2938     2938           
  Misses        151      151           

Impacted Files	Coverage Δ
skll/data/readers.py	90.13% <100.00%> (ø)

---

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 3dafc5f...2daaa24. Read the comment docs.

[mulhod]

This PR removes over 450 lines from the tests!

[desilinguist]

This is fantastic, @mulhod! Thanks for taking this on – this is a really nice improvement of the codebase.

I made a couple of minor suggestions. I also have a couple of broader suggestions:

1. Can you run the tests locally to make sure that all the created files are actually being cleaned up properly? As long as we are working on this, it would be good to make sure of those.

2. I am a little concerned about changing the from tests.utils import X statements to from .utils import X in terms of the impact this will have on the testing of the PyPi and conda packages. In those scenarios, we install the conda package, do a sparse checkout of the skll tests and then run the tests. I think you should be able to test this by making a local conda package and simulating this scenario.

Thanks again!

[mulhod]

This is fantastic, @mulhod! Thanks for taking this on – this is a really nice improvement of the codebase.

I made a couple of minor suggestions. I also have a couple of broader suggestions:

1. Can you run the tests locally to make sure that all the created files are actually being cleaned up properly? As long as we are working on this, it would be good to make sure of those.
2. I am a little concerned about changing the from tests.utils import X statements to from .utils import X in terms of the impact this will have on the testing of the PyPi and conda packages. In those scenarios, we install the conda package, do a sparse checkout of the skll tests and then run the tests. I think you should be able to test this by making a local conda package and simulating this scenario.

Thanks again!

Thanks, @desilinguist! I decided to revert the parts that used relative imports for now. It's a minor change, anyway. I have run all the tests and fixed a couple cases (one that you pointed out!). I think most of the cases were actually issues with the tests that didn't have anything to do with my changes. But, this gave me the chance to really see what files are left behind, so I just fixed any I saw.

[desilinguist]

Thanks so much for doing that @mulhod !

[desilinguist]

LGTM! 👍