Improvements to test suite and code coverage #175
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #175 +/- ##
============================================
+ Coverage 34.01% 37.55% +3.54%
============================================
Files 259 258 -1
Lines 70289 71543 +1254
Branches 13021 13407 +386
============================================
+ Hits 23906 26871 +2965
+ Misses 41879 39729 -2150
- Partials 4504 4943 +439
Help us with your feedback. Take ten seconds to tell us how you rate us. |
|
The testing is reporting as unfinished because This may need an admin override, and the "new" renamed test should be added to the GitHub verification. Maybe we could also prematurely add it to the requirements, although this will disrupt other PRs. |
adcroft
reviewed
Jul 29, 2022
|
build-test-nans was removed so why is it saying "expected - waiting or status to be reported"? Is it because this is a PR and the target does have that test?
|
It was removed from this PR but GitHub is still configured to look for the old one. BTW I think what is currently happening:
|
|
Yes, we'll change the list of required tests when this is merged |
marshallward
force-pushed
the
cov_ci_fixes
branch
from
29bb4dd to
b828689
Compare
adcroft
approved these changes
Jul 29, 2022
Primarily changes to improve coverage reporting, but some other changes
as well:
* New separate rules for `codecov` and `report.cov`
* REPORT_COVERAGE was split into two flags:
* DO_COVERAGE: Enable code coverage reporting
* REQUIRE_COVERAGE_UPLOAD: If true, error if upload fails
* We now report codecov upload failures in the log, but only raise an
error (i.e. nonzero return code) if REQUIRE_COVERAGE_UPLOAD is true.
* GitHub Actions now only reports failed uploads to CodeCov for pull
requests.
* Separate FCFLAGS_FMS flag to control FCFLAGS for FMS builds
- also renamed the old FCFLAGS_FMS to FCFLAGS_DEPS, which was
previously used to pass the flags to the FMS library/modfiles
* $(DEPS) was dropped, and we just use `deps` now.
* Updated the header instructions
* README update
* Codecov.io and GitHub Actions CIs were updated to support changes
Not sure if this is wise, but codecov is severely screwing up these entries.
Updates .testing/Makefile to allow for alternative executable names and multiple executables in one build directory. Until now, the .testing/Makefile assumes all executables are called "MOM6". With the introduction of makedep, executables are called by the name indicated by the Fortran program statement. Changes: - BUILDS used to be a list of directories under build/ . Now is a list of <directory>/<exec_name>. - UNIT_EXECS lists possible executables within build/unit . This list allows us to override targets at the command-line. - Replaces many $(foreach b,$(BUILDS), ...) constructs in favor of build/%/... rules. i.e. reduces use of $(BUILDS) in general - Corrected name of executable in build/unit Co-authored-by: Marshall Ward <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes multiple improvements to the test suite related to code coverage
Failed code coverage uploads to codecov.io now only issue an error for pull requests. Uploads are still attempted, but are not reported as failures if the upload fails for any reason.
In the Makefile, this is controlled by the
REQUIRE_COVERAGE_UPLOADflag parameter.The README and inline documentation of
.testing/Makefilehave been overhauled to reflect many of the recent changes.The asterisk syntax of gcov reports is now removed before upload to codecov.io.
Although the asterisk is a useful indicator of partial coverage, these lines were being ignored by codecov.io and unreported. This caused greater confusion when another report had zero coverage, indicating incorrectly that the line was uncovered. By removing the asterisk, and relying on other metadata to indicate partial coverage, the codecov.io report accuracy now matches the gcov output.
Separate rules for generating and reporting code coverage
It is now much simpler to generate code coverage reports locally, and issues related to uploading to codecov.io can now be ignored.
Makefile executables with names other than
MOM6are now accommodated..testing/Makefileflag overhaulSeveral uncommon flags were removed or renamed, and other new flags were added. As before, these are mostly for internal use and should not affect users. Examples:
FCFLAGS_FMSnow defines the Fortran flags used to build FMS, rather than the flags needed by MOM6 to use FMS. Previously, it was not possible to independently set the FMS flags.FCFLAGS_DEPSnow contains the flags required to access depenencies (i.e. FMS)DEPS, which used to indicate the path to the dependencies, has been removed, and is replaced withdeps.