PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
200 stars 231 forks source link

GitHub Actions to check Unit Test Coverage for each package #3206

Open meetagrawal09 opened 11 months ago

meetagrawal09 commented 11 months ago

Description

I would like to propose adding an action item about checking unit test coverage for packages on CI. This is a step towards always having packages cross a specific percentage of test coverage whenever new functions are added to them. This will enforce (up to some degree) the author of a new function to add tests (might be basic) for the newly added function(s). The step is necessary to always maintain package functions reliable : )

Proposed Solution

I have identified 2 approaches for this :

  1. Having line coverage as a metric. Using the covr package we can get line coverage percentages for each package which can help in deciding on how well the package has been tested.

  2. Having function coverage as a metric. Here we use file names (or file contents) in the tests/testthat directory to decide on how good the test coverage is.

Alternatives Considered

The reviewer can ask the PR author to add tests for the new functions that he/she might have added to the package(s). But that just increases the job of the reviewer.

meetagrawal09 commented 11 months ago

I would vote for the second solution (function coverage). This is because functions do have unit tests which get skipped on CI because they need an additional setup to execute for e.g tests dependent on BETYdb. These tests won't be run by the covr package and so will not be considered while calculating line coverage.

Aariq commented 10 months ago

I would vote for the second solution (function coverage). This is because functions do have unit tests which get skipped on CI because they need an additional setup to execute for e.g tests dependent on BETYdb. These tests won't be run by the covr package and so will not be considered while calculating line coverage.

That's actually a good reason for using covr—as motivation to eventually get tests that can be run on CI (e.g. with database mocking #3066)

Gmin2 commented 6 months ago

@Aariq @meetagrawal09 can i work on adding covr in the ci?

meetagrawal09 commented 6 months ago

@Min2who, thanks for showing your interest in contributing to the issue. I am working on improving our tests without which covr will always fail in CI. Just a side note : Try picking up issues that do not have a Topic : Discussion tag to them since these are not yet ready for implementation and rather just to take a community input on the subject.

Gmin2 commented 6 months ago

@meetagrawal09 can i work on test of this files shiny/Data-Ingest/ui_files