search

nteract / bookstore

📚 Notebook storage and publishing workflows for the masses
https://bookstore.readthedocs.io
BSD 3-Clause "New" or "Revised" License
202 stars 23 forks source link

Test nb_client module #136

Closed mpacer closed 5 years ago

mpacer commented 5 years ago

This add unit tests for the record types and simple functions in nb_client.

It will either take some clever mocking or proper integration tests to properly handle the rest of the classes in this module.

Going through this made me wonder if we should split this module into an nb_types and and nb_client module… as this seemed to be a fairly clean delineation between the two types of functionality.

todo[bot] commented 5 years ago

once id is re-added to NotebookSession's attributes readd this

https://github.com/nteract/bookstore/blob/60249b21cb765ad81d26fa1148733fb79e7efe58/bookstore/client/tests/test_nb_client.py#L93-L98

This comment was generated by todo based on a TODO comment in 60249b21cb765ad81d26fa1148733fb79e7efe58 in #136. cc @mpacer.
codecov[bot] commented 5 years ago

Codecov Report

Merging #136 into master will decrease coverage by 1.23%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
- Coverage   47.32%   46.09%   -1.24%     
==========================================
  Files          11       11              
  Lines         374      384      +10     
==========================================
  Hits          177      177              
- Misses        197      207      +10
codecov[bot] commented 5 years ago

Codecov Report

Merging #136 into master will increase coverage by 18.34%. The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master    #136       +/-   ##
==========================================
+ Coverage   50.25%   68.6%   +18.34%     
==========================================
  Files          11      10        -1     
  Lines         386     395        +9     
==========================================
+ Hits          194     271       +77     
+ Misses        192     124       -68
todo[bot] commented 5 years ago

once id is re-added to NotebookSession's attributes readd this

https://github.com/nteract/bookstore/blob/1c7a9b70486b581df3526ee7c5142f35cfacff12/bookstore/client/tests/test_nb_client.py#L93-L98

This comment was generated by todo based on a TODO comment in 1c7a9b70486b581df3526ee7c5142f35cfacff12 in #136. cc @mpacer.
mpacer commented 5 years ago

There's always something super disappointing when you see a code coverage report for a testing PR that reports a drop in test coverage for some reason…

I don't get it — the tests get picked up by pytest-cov locally:

---------- coverage: platform darwin, python 3.6.8-final-0 -----------
Name                               Stmts   Miss  Cover
------------------------------------------------------
bookstore/__init__.py                 10      1    90%
bookstore/_version.py                 10      0   100%
bookstore/archive.py                  49     11    78%
bookstore/bookstore_config.py         17      0   100%
bookstore/client/__init__.py           1      0   100%
bookstore/client/nb_client.py        112     25    78%
bookstore/client/store_client.py      26     18    31%
bookstore/clone.py                    82     18    78%
bookstore/handlers.py                 28     14    50%
bookstore/publish.py                  38     25    34%
bookstore/s3_paths.py                  9      0   100%
bookstore/utils.py                    12      0   100%
bookstore/validation.py               12     12     0%
------------------------------------------------------
TOTAL                                406    124    69%
todo[bot] commented 5 years ago

once id is re-added to NotebookSession's attributes readd this

https://github.com/nteract/bookstore/blob/0bf64abdf1545e191dd35e65d8608f0ed8c12363/bookstore/client/tests/test_nb_client.py#L93-L98

This comment was generated by todo based on a TODO comment in 0bf64abdf1545e191dd35e65d8608f0ed8c12363 in #136. cc @mpacer.
todo[bot] commented 5 years ago

once id is re-added to NotebookSession's attributes readd this

https://github.com/nteract/bookstore/blob/f72a1bd37f325aa464603b0305c4d64aa8980306/bookstore/client/tests/test_nb_client.py#L93-L98

This comment was generated by todo based on a TODO comment in f72a1bd37f325aa464603b0305c4d64aa8980306 in #136. cc @mpacer.
todo[bot] commented 5 years ago

once id is re-added to NotebookSession's attributes readd this

https://github.com/nteract/bookstore/blob/f72a1bd37f325aa464603b0305c4d64aa8980306/bookstore/client/tests/test_nb_client.py#L93-L98

This comment was generated by todo based on a TODO comment in f72a1bd37f325aa464603b0305c4d64aa8980306 in #136. cc @mpacer.
mpacer commented 5 years ago

Thank you for the reviews @willingc! I think I addressed them all, please let me know if not.

willingc commented 5 years ago

Fantastic @mpacer. Happy to merge.