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.

Sign up for GitHub

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

Jump to bottom

Added code coverage tracking to github action #378

Open
wants to merge 9 commits into
base: devel
Choose a base branch
from
Open

Added code coverage tracking to github action #378

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
71614bd
Added code coverage tracking to github action
caleb-sitton-inl Aug 2, 2024
ce3d555
Reworked total coverage report generation
caleb-sitton-inl Sep 2, 2024
96f3b1a
Added script to enable simple coverage report from github action
caleb-sitton-inl Sep 20, 2024
9c6bc7f
Debugging coverage scripts
caleb-sitton-inl Sep 23, 2024
9df92e4
Edits to coverage reporting structure and created directory
caleb-sitton-inl Sep 27, 2024
9c8feba
Fixing bug
caleb-sitton-inl Sep 28, 2024
1fed821
Additional debugging
caleb-sitton-inl Sep 28, 2024
f2a5787
Taking care of one more bug
caleb-sitton-inl Sep 28, 2024
c543d79
Addressing comments
caleb-sitton-inl Oct 3, 2024
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
17 changes: 15 additions & 2 deletions .github/workflows/github-actions.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,20 @@ jobs:
- run: WD=`(cd ../../.. && pwd)` && export RAVEN_LIBS_NAME="raven_libs_"`basename $WD` && ../raven/scripts/establish_conda_env.sh --install
- run: cd ../raven && ./build_raven
- run: ../raven/run_tests --library-report
- run: ../raven/run_tests -j4 --plugins --re=HERON
# The overhead time added by checking coverage is currently about 19% for a single run_tests.
# Reducing the frequency of coverage checks may be preferable if this increases.
# report_py_coverage is being called twice, once within check_py_coverage to print to the terminal and once here to get data for the annotation
- run: >
bash coverage_scripts/check_py_coverage.sh -j4 &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

so all of these commands have to be within the same run command because they rely on having the same environment variable definitions, I presume. all of the &&s get a little confusing, at least for me, so I think setting some variables at the top of this .yml might help clean up the code? That way you can refer to things like DATA_FILE without having to define it all within this run job. See the env section defined above the jobs section here: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#defining-environment-variables-for-a-single-workflow

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 put them all in the same run mostly because they're related, so I just assumed that the same run would be best for stylistic reasons and just to avoid any potential errors from splitting them up that I might not spot. I agree that it makes more sense to split it though if possible. I can definitely move the DATA_FILE and COV_RCFILE definitions to somewhere more out of the way using the env section. I think I can separate the check_py_coverage line out into its own run job as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On second thought, since DATA_FILE and COV_RCFILE definitions both use pwd, I think I do actually need to define them in the same run step as they're used. The check_py_coverage can still be moved to a different run step though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, I would assume that pwd would give about the same value each time (if not, other things would fail I think).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The issue is that, to be accessible where they're used, the environment variables need to either be defined over an entire section with env or defined within the same run, and in env, I don't believe you can run bash commands, including pwd.

DATA_FILE=`pwd`/.coverage && COV_RCFILE=`pwd`/coverage_scripts/.coveragerc &&
COV_RPT=`bash coverage_scripts/report_py_coverage.sh --data-file=$DATA_FILE --coverage-rc-file=$COV_RCFILE` &&
echo "::notice title=Coverage Summary::$COV_RPT For details, download 'coverage_results' from Artifacts, extract all files, and open 'index.html'."
- name: Archive coverage results
uses: actions/upload-artifact@v4
if: always()
with:
name: coverage_results
path: tests/coverage_html_report
Test-HERON-Windows:
runs-on: [self-hosted, windows]
steps:
Expand All @@ -33,4 +46,4 @@ jobs:
- run: $Env:RAVEN_LIBS_NAME = "raven_libraries_"+(Get-Location).Path.Split("\")[-4]; bash ../raven/scripts/establish_conda_env.sh --install --conda-defs $HOME/Miniconda3/etc/profile.d/conda.sh
- run: cd ../raven; bash ./build_raven
- run: bash ../raven/run_tests --library-report
- run: bash ../raven/run_tests -j4 --plugins --re=HERON
- run: bash ../raven/run_tests -j4 --plugins --re=HERON/tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to confirm -- this doesn't change any functionality, it just cleans up the reporting? since there are no tests run outside of the HERON/tests directory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, because of some strange paths in the github runner, with the previous version all plugin tests for HERON, TEAL, and the ExamplePlugin were being run. This limits it to only the HERON tests.

0 tests/.coveragerc → coverage_scripts/.coveragerc
View file
Open in desktop
File renamed without changes.
34 changes: 34 additions & 0 deletions coverage_scripts/check_py_coverage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash
SCRIPT_DIRNAME=`dirname $0`
HERON_DIR=`(cd $SCRIPT_DIRNAME/..; pwd)`
cd $HERON_DIR
RAVEN_DIR=`python -c 'from src._utils import get_raven_loc; print(get_raven_loc())'`

source $HERON_DIR/coverage_scripts/initialize_coverage.sh

#coverage help run
SRC_DIR=`(cd src && pwd)`

# get display var
DISPLAY_VAR=`(echo $DISPLAY)`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this not just DISPLAY_VAR=$DISPLAY. or possibly DISPLAY_VAR=${DISPLAY:-default} if you need a default value.

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 don't actually have a great understanding of the importance of the DISPLAY manipulation. This was a carryover from the check_py_coverage script in RAVEN on which this script is based. After studying it some, I don't think it's necessary, and it does not have an effect on the functionality of the script, at least locally, so I'll plan to go ahead and remove lines 12-15 and 26-27.

# reset it
export DISPLAY=

export COVERAGE_RCFILE="$SRC_DIR/../coverage_scripts/.coveragerc"
SOURCE_DIRS=($SRC_DIR,$SRC_DIR/../templates/)
OMIT_FILES=($SRC_DIR/dispatch/twin_pyomo_test.py,$SRC_DIR/dispatch/twin_pyomo_test_rte.py,$SRC_DIR/dispatch/twin_pyomo_limited_ramp.py,$SRC_DIR/ArmaBypass.py)
EXTRA="--source=${SOURCE_DIRS[@]} --omit=${OMIT_FILES[@]} --parallel-mode "
export COVERAGE_FILE=`pwd`/.coverage

coverage erase
($RAVEN_DIR/run_tests "$@" --re=HERON/tests --python-command="coverage run $EXTRA " || echo run_tests done but some tests failed)

#get DISPLAY BACK
DISPLAY=$DISPLAY_VAR

## Prepare data and generate the html documents
coverage combine
coverage html

# See report_py_coverage.sh file for explanation of script separation
(bash $HERON_DIR/coverage_scripts/report_py_coverage.sh --data-file=$COVERAGE_FILE --coverage-rc-file=$COVERAGE_RCFILE)
37 changes: 7 additions & 30 deletions check_py_coverage.sh → coverage_scripts/initialize_coverage.sh
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
#!/bin/bash

# This script prepares for running commands from the coverage package

SCRIPT_DIRNAME=`dirname $0`
SCRIPT_DIR=`(cd $SCRIPT_DIRNAME; pwd)`
HERON_DIR=`(cd $SCRIPT_DIRNAME/..; pwd)`
echo $HERON_DIR
cd $HERON_DIR
RAVEN_DIR=`python -c 'from src._utils import get_raven_loc; print(get_raven_loc())'`
source $RAVEN_DIR/scripts/establish_conda_env.sh --quiet --load
RAVEN_LIBS_PATH=`conda env list | awk -v rln="$RAVEN_LIBS_NAME" '$0 ~ rln {print $NF}'`
BUILD_DIR=${BUILD_DIR:=$RAVEN_LIBS_PATH/build}
INSTALL_DIR=${INSTALL_DIR:=$RAVEN_LIBS_PATH}
PYTHON_CMD=${PYTHON_CMD:=python}
JOBS=${JOBS:=1}
mkdir -p $BUILD_DIR
mkdir -p $INSTALL_DIR
DOWNLOADER='curl -C - -L -O '
Expand All @@ -23,7 +27,7 @@ update_python_path ()
}

update_python_path
PATH=$INSTALL_DIR/bin:$PATH
export PATH=$INSTALL_DIR/bin:$PATH

if which coverage
then
Expand All @@ -45,30 +49,3 @@ else
fi

update_python_path

cd $SCRIPT_DIR

#coverage help run
SRC_DIR=`(cd src && pwd)`

# get display var
DISPLAY_VAR=`(echo $DISPLAY)`
# reset it
export DISPLAY=

export COVERAGE_RCFILE="$SRC_DIR/../tests/.coveragerc"
SOURCE_DIRS=($SRC_DIR,$SRC_DIR/../templates/)
OMIT_FILES=($SRC_DIR/dispatch/twin_pyomo_test.py,$SRC_DIR/dispatch/twin_pyomo_test_rte.py,$SRC_DIR/dispatch/twin_pyomo_limited_ramp.py,$SRC_DIR/ArmaBypass.py)
EXTRA="--source=${SOURCE_DIRS[@]} --omit=${OMIT_FILES[@]} --parallel-mode "
export COVERAGE_FILE=`pwd`/.coverage

coverage erase
($RAVEN_DIR/run_tests "$@" --re=HERON/tests --python-command="coverage run $EXTRA " || echo run_tests done but some tests failed)

#get DISPLAY BACK
DISPLAY=$DISPLAY_VAR

## Prepare data and generate the html documents
coverage combine
coverage html

39 changes: 39 additions & 0 deletions coverage_scripts/report_py_coverage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/bin/bash

# To be run after a run of check_py_coverage.sh
# This script has been separated from check_py_coverage.sh for the github action, so that
# the output of run_tests within check_py_coverage.sh can be printed, but the report
# value can be caught and put in an annotation. This is necessary because calling
# "coverage report --format=total" directly in the yaml does not work

SCRIPT_DIRNAME=`dirname $0`
HERON_DIR=`(cd $SCRIPT_DIRNAME/..; pwd)`
cd $HERON_DIR

source coverage_scripts/initialize_coverage.sh > /dev/null 2>&1

# read command-line arguments
ARGS=()
for A in "$@"
do
case $A in
--data-file=*)
export COVERAGE_FILE="${A#--data-file=}" # Removes "--data-file=" and puts path into env variable
;;
--coverage-rc-file=*)
export COVERAGE_RCFILE="${A#--coverage-rc-file=}" # See above
;;
*)
ARGS+=("$A")
;;
esac
done

COV_VAL=`coverage report --format=total "${ARGS[@]}"`
if [[ $COV_VAL = "No data to report." ]]
then
echo "Could not find data file with coverage results."
exit 0
fi

echo "Coverage for this repository is now $COV_VAL%."
Loading