MicroMedIAn / PathAIA

Digital Pathology Analysis Tools

GNU General Public License v3.0

3 stars 3 forks source link

Unit testing #18

Closed ArnaudAbreu closed 3 years ago

ArnaudAbreu commented 3 years ago

Unit tests:

test package with simple test module
FakeSlide object to enable testing without having actual slide in repo
test slide_rois function
tox testing enabled
github action for tox testing with code coverage on every push and pull request
updated link to pathaia project on codecov

ArnaudAbreu commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@da8df2a). Click here to learn what that means. The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #18   +/-   ##
=========================================
  Coverage          ?   36.58%           
=========================================
  Files             ?       14           
  Lines             ?      708           
  Branches          ?        0           
=========================================
  Hits              ?      259           
  Misses            ?      449           
  Partials          ?        0

Flag	Coverage Δ
unittests	`36.58% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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 da8df2a...596eb6b. Read the comment docs.

schwobr commented 3 years ago

Everything looks fine to me, I just have few remarks / questions:

Couldn't we put HE_MASK in a separate file to help readibility ? We could put all global objects defined for testing in a file and rest of test code in a separate file.
I don't remember much of good test practices, but shouldn't the test folder mimic the library folder's structure ? Maybe you already had that in mind.
For now you get the codecov badge from this branch but we should remember to put it back on master branch once this PR is merged (or right before merging).
Do we create all tests in this branch before merging ? Or do we merge that right now and create additional tests after ? Both are fine imo, but maybe merging this now is better so we have the full project backbone ready on master. I'll start working on writing other tests right after.

ArnaudAbreu commented 3 years ago

Thanks,

Yes, let's put HE_MASK somewhere else, I agree
For the test folder I guess the reason of having the same structure is not to get lost when writing and finding the tests, but I don't see any other reason since test tools find every test files in the projects by themselves.
Yes, it was just for testing the badge.
I think we should merge now and add more tests later.