Repository navigation
Include test files in coverage report #6902
Description
Of course I can run coverage for tests locally, but I think it should be accessible on coveralls. You cannot blindly rely on a test report until you are sure that they are actually working, otherwise it could give you a false sense of security.
I have faced a problem where I cannot be sure that pytest runs all the tests. Moreover I have got a test failures with pytest on tests that nose does not perform (and I do not why then they are not deleted). Here is the list:
- test_sankey not deprecated, runs without issues
- test_skew not deprecated, has outdated baseline images or tolerance problems (
..FF.F: RMS 0.014, 0.264, 0.065) - test_ttconv not deprecated, runs without issues
- test_usetex not deprecated, has outdated baseline images or tolerance problems (
.F: RMS 0.261)
What benefits can we have by including test files coverage in the report? I collected dark spots in the test suite:
Important (test not running, uncovered cases):
- test_animation:test_no_length_frames
- test_axes:test_pcolorargs
- test_backend_pdf:test_use14corefonts does not run on travis (
TRAVISenv variable was removed?) - test_contour:test_contour_manual_labels
- test_image:test_minimized_rasterized
- test_mlab:Test_detrend.test_detrend_mean_2D_none
- test_mlab:Test_detrend.test_detrend_mean_2D_none_T
- test_mlab:Test_detrend.test_detrend_mean_2D_axis0
- test_mlab:Test_detrend.test_detrend_mean_2D_axis1
- test_mlab:Test_detrend.test_detrend_mean_2D_axism1
- test_mlab:Test_detrend.test_detrend_str_linear_2d_slope_off_axis0
- test_mlab:Test_detrend.test_detrend_detrend_linear_1d_slope_off_axis1
- test_mlab:test_griddata_nn
- test_subplots:test_shared
- test_patheffects:test_PathEffect_points_to_pixels
Less important (partial coverage):
- test_backend_svg _test_determinism
- test_cbook:test_results_whiskers_range
- test_cbook:test_results_whiskers_percentiles
- test_dates:test_date2num_dst
test_delaunay- test_mlab:Test_window
- test_mlab:Test_spectral_nosig_real_onesided
- test_units:test_numpy_facade
Not important (requires tests for the tests, and other cases):
- test_collections check_allprop unused function?
- test_collections check_segments
- test_transforms:test_non_affine_caching transform_path_non_affine&transform_non_affine
- test_triplot_return
Someone can say that we need tests for the tests to have full coverage -- I would say no, some special places can be marked with #pragma: no cover, but other can be fixed to have full coverage.
Currently this can be done with nose-cov plugin and after migrating to pytest we will have exactly same coverage reports with pytest-cov.
Activity
jenshnielsen commentedon Aug 7, 2016
Thanks for investigating this. I think the fundamental problem is that we have an opt in
default_test_modulesinmatplotlib/__init__.pyand new test modules have been added without adding them to that list. I suggest that we get rid of that mechanism. If tests need to be skipped they should be explicitly marked as such. This covers your first category.I am not convinced that adding the tests to the default coverage is the right solution. To me it would bias the coverage towards an artificially high number which is not really representative of the real coverage of our code.
Kojoley commentedon Aug 7, 2016
I am for this too.
Currently for pytest I have developed two filters:
--collect-filter=whitelist(default) uses thedefault_test_moduleslist--collect-filter=blacklistuses the list of disabled tests (actually this can be done withdef setup_module: skip()), but I have done it to support backward comparability with nose--collect-filter=nonedo not use any filtersBut I will drop them with pleasure.
Currently code coverage percentage is already faked (for example there are only 3 backends in coverage report, while other do not included, because they have zero coverage). I do not think anyone measures matplotlib by coverage number, but test code is a code too, and it is better to have overestimated score than tests which do not work.
jenshnielsen commentedon Aug 7, 2016
I definitely support getting rid of the whitelisting as a option or at least not making it the default. I think our experience have shown that its very error prone.
The backends are definitely an issue for testing. That is the main driver behind deprecating the WX non AGG backend and the GDK backend. We do most definitely need more coverage for the Cairo, ps, pgf backends and the various interactive aspects of GUI toolkits.
That being said the backends are not excluded from coverage it just happens that the python code that implements most backends is a small fraction of the total code. You cannot imply that we need to run all examples with all backends to get full coverage since most of the tests are testing non backend specific code.
Kojoley commentedon Aug 7, 2016
I cannot say that including tests in coverage report is a definitely good solution, but until there is no other ways (or I do not know about them) to prevent issues such like #6839 (and others mentioned above) I am for it.
Add tests previously left out workaround for matplotlib#6902
Add tests previously left out workaround for matplotlib#6902
story645 commentedon Aug 23, 2016
So the way coverage is currently set up, anything using py.test tanks miserably: https://coveralls.io/jobs/17551007 (eta: it's an issue for #6889)
QuLogic commentedon Feb 5, 2017
Fixed by #8003.