search

openzim / python-libzim

Libzim binding for Python: read/write ZIM files in Python
https://pypi.org/project/libzim/
GNU General Public License v3.0
62 stars 20 forks source link

Added tests to ensure Article's methods all raises if not overriden #67

Closed rgaudin closed 4 years ago

codecov[bot] commented 4 years ago

Codecov Report

Merging #67 into master will increase coverage by 9.37%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #67      +/-   ##
===========================================
+ Coverage   90.62%   100.00%   +9.37%     
===========================================
  Files           2         2              
  Lines          96        96              
===========================================
+ Hits           87        96       +9     
+ Misses          9         0       -9
Impacted Files Coverage Δ
libzim/writer.py 100.00% <0.00%> (+9.57%) :arrow_up:

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 cd4d3b3...178b4bc. Read the comment docs.

mgautierfr commented 4 years ago

This seems a bit too complex. Especially, you are testing that not implementing a method correctly raise an exception by implementing the method and calling the "super" implementation.

Pytest has a fixture to monkeypath a object (and remove a method/attribute) https://docs.pytest.org/en/stable/monkeypatch.html It is probably better to use it to remove method from a correctly implemented article.

And is is better to move the method to remove to a parametrized fixture (https://docs.pytest.org/en/stable/fixture.html#parametrizing-fixtures) instead of having a loop in the test.

rgaudin commented 4 years ago

You're right it's clearer. @mgautierfr can you take another look?