Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make code coverage report available #2287

Merged
merged 26 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
67ed723
run pytest from GHA environment instead of through Use Case Test acti…
georgemccabe Aug 2, 2023
51588cb
set env var needed to run pytests
georgemccabe Aug 2, 2023
952dc15
add lcov report
georgemccabe Aug 2, 2023
57c45f9
pass coverage config file to pytest command
georgemccabe Aug 2, 2023
4bda6d9
pass github token
georgemccabe Aug 2, 2023
18d0fe2
call coverage to run pytests
georgemccabe Aug 2, 2023
3a4211f
try running with lcov output
georgemccabe Aug 2, 2023
d4a2c56
moved file back to top level
georgemccabe Aug 2, 2023
cc1e640
use better shell syntax
georgemccabe Aug 2, 2023
4e505be
try again
georgemccabe Aug 2, 2023
af99699
run debug
georgemccabe Aug 2, 2023
a2ba6b7
remove token and test if git is available
georgemccabe Aug 2, 2023
bdd1455
remove git test
georgemccabe Aug 2, 2023
060e08c
try develop versiong
georgemccabe Aug 2, 2023
e258e27
add token using dev
georgemccabe Aug 2, 2023
feb6b88
try with lcov
georgemccabe Aug 2, 2023
c4f3caf
set source in coverage config
georgemccabe Aug 2, 2023
4597024
run coverage then generate report
georgemccabe Aug 2, 2023
2977a3b
split pytest run and coverage report into separate steps to catch fai…
georgemccabe Aug 2, 2023
c83575f
force tests to fall to ensure that failure is properly captured
georgemccabe Aug 2, 2023
0d354c0
generate coverage report even if a test fails
georgemccabe Aug 2, 2023
09a1317
require unit tests to pass to run use case tests
georgemccabe Aug 2, 2023
6f69249
turn on use case group to test
georgemccabe Aug 2, 2023
3aa3c6e
remove force failure of unit test to test that use case will run
georgemccabe Aug 2, 2023
ca2eb5f
turn off use case after test
georgemccabe Aug 2, 2023
109316a
move helper script that is only used internally to internals directory
georgemccabe Aug 2, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[run]
relative_files = True
source = metplus
42 changes: 5 additions & 37 deletions .github/actions/run_tests/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ source ${GITHUB_WORKSPACE}/${CI_JOBS_DIR}/bash_functions.sh

# get branch name for push or pull request events
# add -pull_request if pull request event to keep separated
branch_name=`${GITHUB_WORKSPACE}/${CI_JOBS_DIR}/print_branch_name.py`
branch_name=$(${GITHUB_WORKSPACE}/${CI_JOBS_DIR}/print_branch_name.py)
if [ "$GITHUB_EVENT_NAME" == "pull_request" ]; then
branch_name=${branch_name}-pull_request
fi
Expand All @@ -23,57 +23,25 @@ time_command docker pull $DOCKERHUBTAG

# if unsuccessful (i.e. pull request from a fork)
# then build image locally
docker inspect --type=image $DOCKERHUBTAG > /dev/null
if [ $? != 0 ]; then
if ! docker inspect --type=image $DOCKERHUBTAG > /dev/null; then
# if docker pull fails, build locally
echo docker pull failed. Building Docker image locally...
${GITHUB_WORKSPACE}/${CI_JOBS_DIR}/docker_setup.sh
fi

# running unit tests (pytests)
if [[ "$INPUT_CATEGORIES" == pytests* ]]; then
export METPLUS_ENV_TAG="test.v5.1"
export METPLUS_IMG_TAG=${branch_name}
echo METPLUS_ENV_TAG=${METPLUS_ENV_TAG}
echo METPLUS_IMG_TAG=${METPLUS_IMG_TAG}

export RUN_TAG=metplus-run-env

# use BuildKit to build image
export DOCKER_BUILDKIT=1

start_seconds=$SECONDS

# build an image with the pytest conda env and the METplus branch image
# Note: adding --build-arg <arg-name> without any value tells docker to
# use value from local environment (export METPLUS_IMG_TAG)
time_command docker build -t $RUN_TAG \
--build-arg METPLUS_IMG_TAG \
--build-arg METPLUS_ENV_TAG \
-f .github/actions/run_tests/Dockerfile.run \
.

echo Running Pytests
command="export METPLUS_TEST_OUTPUT_BASE=/data/output;"
command+="/usr/local/conda/envs/${METPLUS_ENV_TAG}/bin/pytest internal/tests/pytests -vv --cov=metplus --cov-append --cov-report=term-missing;"
command+="if [ \$? != 0 ]; then echo ERROR: Some pytests failed. Search for FAILED to review; false; fi"
time_command docker run -v $WS_PATH:$GITHUB_WORKSPACE --workdir $GITHUB_WORKSPACE $RUN_TAG bash -c "$command"
exit $?
fi

# running use case tests

# split apart use case category and subset list from input
CATEGORIES=`echo $INPUT_CATEGORIES | awk -F: '{print $1}'`
SUBSETLIST=`echo $INPUT_CATEGORIES | awk -F: '{print $2}'`
CATEGORIES=$(echo $INPUT_CATEGORIES | awk -F: '{print $1}')
SUBSETLIST=$(echo $INPUT_CATEGORIES | awk -F: '{print $2}')

# run all cases if no subset list specified
if [ -z "${SUBSETLIST}" ]; then
SUBSETLIST="all"
fi

# get METviewer if used in any use cases
all_requirements=`./${CI_JOBS_DIR}/get_requirements.py ${CATEGORIES} ${SUBSETLIST}`
all_requirements=$(./${CI_JOBS_DIR}/get_requirements.py ${CATEGORIES} ${SUBSETLIST})
echo All requirements: $all_requirements
NETWORK_ARG=""
if [[ "$all_requirements" =~ .*"metviewer".* ]]; then
Expand Down
17 changes: 0 additions & 17 deletions .github/jobs/get_use_cases_to_run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ matrix="[]"

run_use_cases=$1
run_all_use_cases=$2
run_unit_tests=$3

echo Run use cases: $run_use_cases
echo Run all use cases: $run_all_use_cases
echo Run unit tests: $run_unit_tests

# if running use cases, generate JQ filter to use
if [ "$run_use_cases" == "true" ]; then
Expand All @@ -28,21 +26,6 @@ if [ "$run_use_cases" == "true" ]; then

fi

# if unit tests will be run, add "pytests" to beginning of matrix list
if [ "$run_unit_tests" == "true" ]; then
echo Adding unit tests to list to run

pytests="\"pytests\","

# if matrix is empty, set to an array that only includes pytests
if [ "$matrix" == "[]" ]; then
matrix="[${pytests:0: -1}]"
# otherwise prepend item to list
else
matrix="[${pytests}${matrix:1}"
fi
fi

echo Array of groups to run is: $matrix
# if matrix is still empty, exit 1 to fail step and skip rest of workflow
if [ "$matrix" == "[]" ]; then
Expand Down
3 changes: 2 additions & 1 deletion .github/jobs/set_job_controls.sh
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ fi
echo "run_get_image=$run_get_image" >> $GITHUB_OUTPUT
echo "run_get_input_data=$run_get_input_data" >> $GITHUB_OUTPUT
echo "run_diff=$run_diff" >> $GITHUB_OUTPUT
echo "run_unit_tests=$run_unit_tests" >> $GITHUB_OUTPUT
echo "run_save_truth_data=$run_save_truth_data" >> $GITHUB_OUTPUT
echo "external_trigger=$external_trigger" >> $GITHUB_OUTPUT

Expand All @@ -96,7 +97,7 @@ branch_name=`${GITHUB_WORKSPACE}/.github/jobs/print_branch_name.py`
echo "branch_name=$branch_name" >> $GITHUB_OUTPUT

# get use cases to run
.github/jobs/get_use_cases_to_run.sh $run_use_cases $run_all_use_cases $run_unit_tests
.github/jobs/get_use_cases_to_run.sh $run_use_cases $run_all_use_cases

# echo output variables to review in logs
echo branch_name: $branch_name
Expand Down
43 changes: 36 additions & 7 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ jobs:
run_get_image: ${{ steps.job_status.outputs.run_get_image }}
run_get_input_data: ${{ steps.job_status.outputs.run_get_input_data }}
run_diff: ${{ steps.job_status.outputs.run_diff }}
run_unit_tests: ${{ steps.job_status.outputs.run_unit_tests }}
run_save_truth_data: ${{ steps.job_status.outputs.run_save_truth_data }}
external_trigger: ${{ steps.job_status.outputs.external_trigger }}
branch_name: ${{ steps.job_status.outputs.branch_name }}
Expand All @@ -87,7 +88,7 @@ jobs:
name: Docker Setup - Get METplus Image
runs-on: ubuntu-latest
needs: job_control
if: ${{ needs.job_control.outputs.run_get_image == 'true' }}
if: ${{ needs.job_control.outputs.run_get_image == 'true' && needs.job_control.outputs.run_some_tests == 'true' }}
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
Expand All @@ -105,7 +106,7 @@ jobs:
name: Docker Setup - Update Data Volumes
runs-on: ubuntu-latest
needs: job_control
if: ${{ needs.job_control.outputs.run_get_input_data == 'true' }}
if: ${{ needs.job_control.outputs.run_get_input_data == 'true' && needs.job_control.outputs.run_some_tests == 'true' }}
continue-on-error: true
steps:
- uses: dtcenter/metplus-action-data-update@v2
Expand All @@ -121,10 +122,38 @@ jobs:
use_feature_data: true
tag_max_pages: 15

unit_tests:
name: Unit Tests
runs-on: ubuntu-latest
needs: [job_control]
if: ${{ needs.job_control.outputs.run_unit_tests == 'true' }}
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: '3.10'
cache: 'pip'
- name: Install Python Test Dependencies
run: |
python3 -m pip install --upgrade pip
python3 -m pip install -r internal/tests/pytests/requirements.txt
- name: Run Pytests
run: coverage run -m pytest internal/tests/pytests
env:
METPLUS_TEST_OUTPUT_BASE: ${{ runner.workspace }}/pytest_output
- name: Generate coverage report
run: coverage report -m
if: always()
- name: Run Coveralls
uses: AndreMiras/coveralls-python-action@develop
if: always()
with:
github-token: ${{ secrets.GITHUB_TOKEN }}

use_case_tests:
name: Use Case Tests
runs-on: ubuntu-latest
needs: [get_image, update_data_volumes, job_control]
needs: [get_image, update_data_volumes, job_control, unit_tests]
if: ${{ needs.job_control.outputs.run_some_tests == 'true' }}
strategy:
fail-fast: false
Expand Down Expand Up @@ -158,24 +187,24 @@ jobs:
# copy logs with errors to error_logs directory to save as artifact
- name: Save error logs
id: save-errors
if: ${{ always() && steps.run_tests.conclusion == 'failure' && !startsWith(matrix.categories,'pytests') }}
if: ${{ always() && steps.run_tests.conclusion == 'failure' }}
run: .github/jobs/save_error_logs.sh

# run difference testing
- name: Run difference tests
id: run-diff
if: ${{ needs.job_control.outputs.run_diff == 'true' && steps.run_tests.conclusion == 'success' && !startsWith(matrix.categories,'pytests') }}
if: ${{ needs.job_control.outputs.run_diff == 'true' && steps.run_tests.conclusion == 'success' }}
run: .github/jobs/run_difference_tests.sh ${{ matrix.categories }} ${{ steps.get-artifact-name.outputs.artifact_name }}

# copy output data to save as artifact
- name: Save output data
id: save-output
if: ${{ always() && steps.run_tests.conclusion != 'skipped' && !startsWith(matrix.categories,'pytests') }}
if: ${{ always() && steps.run_tests.conclusion != 'skipped' }}
run: .github/jobs/copy_output_to_artifact.sh ${{ steps.get-artifact-name.outputs.artifact_name }}

- name: Upload output data artifact
uses: actions/upload-artifact@v3
if: ${{ always() && steps.run_tests.conclusion != 'skipped' && !startsWith(matrix.categories,'pytests') }}
if: ${{ always() && steps.run_tests.conclusion != 'skipped' }}
with:
name: ${{ steps.get-artifact-name.outputs.artifact_name }}
path: artifact/${{ steps.get-artifact-name.outputs.artifact_name }}
Expand Down
6 changes: 3 additions & 3 deletions docs/Contributors_Guide/basic_components.rst
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,21 @@ used to easily add support for overriding MET configuration variables that
were not previously supported in METplus configuration files.

There is a utility that can be used to easily see what changes are needed to
add support for a new variable. The doc_util.py script can be run from the
add support for a new variable. The add_met_config_helper.py script can be run from the
command line to output a list of instructions to add new support. It can
be run from the top level of the METplus repository. The script can be called
to add a single MET configuration variable by supplying the MET tool name and
the variable name::

./metplus/util/doc_util.py point_stat sid_exc
./internal/scripts/dev_tools/add_met_config_helper.py point_stat sid_exc

This command will provide guidance for adding support for the sid_exc variable
found in the PointStatConfig file.

The script can also be called with the name of a dictionary and the names of
each dictionary variable::

./metplus/util/doc_util.py grid_stat distance_map baddeley_p baddeley_max_dist fom_alpha zhu_weight beta_value_n
./internal/scripts/dev_tools/add_met_config_helper.py grid_stat distance_map baddeley_p baddeley_max_dist fom_alpha zhu_weight beta_value_n

This command will provide guidance for adding support for the distance_map
dictionary found in the GridStatConfig file. The list of variables found inside
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@
import os

try:
from .string_manip import get_wrapper_name
from metplus.util.string_manip import get_wrapper_name
except ImportError:
sys.path.insert(0, os.path.abspath(os.path.dirname(__file__)))
from string_manip import get_wrapper_name
# if metplus package is not installed, find util relative to this script
metplus_home = os.path.join(os.path.dirname(__file__),
os.pardir, os.pardir, os.pardir)
sys.path.insert(0, os.path.abspath(metplus_home))
from metplus.util.string_manip import get_wrapper_name

SCRIPT_INFO_TEXT = (
'This script is intended to help developers add support for setting '
Expand Down
19 changes: 19 additions & 0 deletions internal/tests/pytests/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
certifi==2023.7.22

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a requirements.in that is compiled with pip-compile to check on versioning? Specifying the versions in requirements.txt without this (whether it be poetry/pipenv/choose-your poison is prone to error. Using pip-compile (and others) ensures you have all requirements, not just the ones you think you need.

Additionally, why is this dev req hidden so deep in the directory structure? It's dev, but still top-level requirements files are easier to manage/build for your project. Would be beneficial to place it up top with your prod requirements.txt and rename to requirements-dev.txt then you can include requirements.txt in your requirements-dev.txt (if using pip-compile include requirements.in in your requirements-dev.in) and pip will know what to do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @halltristanj,

I have not used pip-compile but after a quick review it does sound like a good tool to help handle dependencies. I generated the file by creating a new environment, installing the packages I needed to run the unit tests, and ran pip freeze.

I created this requirements file to easily set up an environment to run the unit tests in GitHub Actions. I hadn't necessarily intended it to be used by other developers, although that would be nice to provide. It gets complicated because there are different types of contributors that need different optional dependencies for their work. For example, contributors working on the source code or unit tests will want the packages needed to run the unit tests, while contributors adding a new use case may only need the packages needed to build the documentation locally (found in docs/requirements.txt). The METplus Contributor's Guide contains some info to help developers obtain what they need for their work.

The changes in this pull request were added to more easily view the code coverage results because we have external contributors actively working on improving the code coverage of the tests. I appreciate your suggestions and will keep them in mind for when we are able to dedicate more time to improving these things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created GitHub issue #2289 to track this enhancement.

cftime==1.6.2
coverage==7.2.7
exceptiongroup==1.1.2
iniconfig==2.0.0
netCDF4==1.6.4
numpy==1.25.2
packaging==23.1
pandas==2.0.3
pdf2image==1.16.3
Pillow==10.0.0
pluggy==1.2.0
pytest==7.4.0
pytest-cov==4.1.0
python-dateutil==2.8.2
pytz==2023.3
six==1.16.0
tomli==2.0.1
tzdata==2023.3
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ def set_minimum_config_settings(config):
)
@pytest.mark.wrapper_b
def test_grid_stat_is_prob(metplus_config, config_overrides, expected_values):

config = metplus_config

set_minimum_config_settings(config)
Expand Down
1 change: 0 additions & 1 deletion metplus/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from .config_util import *
from .config_metplus import *
from .config_validate import *
from .doc_util import *
from .run_util import *
from .met_config import *
from .time_looping import *
Expand Down