Skip to content

Commit

Permalink
Merge PR ReactionMechanismGenerator#2501. Expand and fix regression t…
Browse files Browse the repository at this point in the history
…est comments.

The results of the regression tests were largely buried in the
logs of the continuous integration action workflow, rather than
being nicely summarized on the annotation to the pull request.

The summaries and comparisons that are generated by the CI running
the regression tests are captured, and used to make the annotation.
The summary is stored as a text file, with the pull request number
stored on the first line. This is then uploaded as an artifact.
A new workflow runs whenever the CI workflow completes, then downloads
 the artifact, extracts the summary, and posts a comment.
 This complicated runaround is needed because workflows run by
 pull requests from forks do not have permission to write comments directly.

Also fixed up some markdown formatting.
  • Loading branch information
rwest committed Jul 22, 2023
2 parents 940a042 + bc9be93 commit 4ee1e71
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 25 deletions.
58 changes: 37 additions & 21 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
# 2023-06-15 - revert changes from 06-06, both now work
# 2023-06-27 - add option to run from RMG-database with GitHub resuable workflows
# 2023-07-17 - made it pass by default
# 2023-07-21 - upload the regression results summary as artifact (for use as a comment on PRs)
name: Continuous Integration

on:
Expand Down Expand Up @@ -269,11 +270,12 @@ jobs:
env:
REFERENCE: stable_regression_results
run: |
exec 2> >(tee -a regression.stderr >&2) 1> >(tee -a regression.stdout)
mkdir -p "test/regression-diff"
for regr_test in aromatics liquid_oxidation nitrogen oxidation sulfur superminimal RMS_constantVIdealGasReactor_superminimal RMS_CSTR_liquid_oxidation;
do
echo ""
echo "## Regression test $regr_test:"
echo "### Regression test $regr_test:"
# Memory Usage and Execution Time
echo -n 'Reference: '
grep "Execution time" $REFERENCE/"$regr_test"/RMG.log | tail -1
Expand All @@ -284,6 +286,7 @@ jobs:
echo -n 'Current: '
grep "Memory used:" test/regression/"$regr_test"/RMG.log | tail -1
echo "<details>"
# Compare the edge and core
if python-jl scripts/checkModels.py \
"$regr_test-core" \
Expand All @@ -292,72 +295,85 @@ jobs:
test/regression/"$regr_test"/chemkin/chem_annotated.inp \
test/regression/"$regr_test"/chemkin/species_dictionary.txt
then
echo "$regr_test Passed Core Comparison"
echo "<summary>$regr_test Passed Core Comparison ✅</summary>"
else
echo "$regr_test Failed Core Comparison" | tee -a $GITHUB_STEP_SUMMARY
echo "<summary>$regr_test Failed Core Comparison ❌</summary>"
cp "$regr_test-core.log" test/regression-diff/
export FAILED=Yes
fi
echo "" # blank line so next block is interpreted as markdown
cat "$regr_test-core.log"
echo "</details>"
echo "<details>"
if python-jl scripts/checkModels.py \
"$regr_test-edge" \
$REFERENCE/"$regr_test"/chemkin/chem_edge_annotated.inp \
$REFERENCE/"$regr_test"/chemkin/species_edge_dictionary.txt \
test/regression/"$regr_test"/chemkin/chem_edge_annotated.inp \
test/regression/"$regr_test"/chemkin/species_edge_dictionary.txt
then
echo "$regr_test Passed Edge Comparison"
echo "<summary>$regr_test Passed Edge Comparison ✅</summary>"
else
echo "$regr_test Failed Edge Comparison" | tee -a $GITHUB_STEP_SUMMARY
echo "<summary>$regr_test Failed Edge Comparison ❌</summary>"
cp "$regr_test-edge.log" test/regression-diff/
export FAILED=Yes
fi
echo "" # blank line so next block is interpreted as markdown
cat "$regr_test-edge.log"
echo "</details>"
# Check for Regression between Reference and Dynamic (skip superminimal)
if [ -f test/regression/"$regr_test"/regression_input.py ];
then
echo "<details>"
if python-jl rmgpy/tools/regression.py \
test/regression/"$regr_test"/regression_input.py \
$REFERENCE/"$regr_test"/chemkin \
test/regression/"$regr_test"/chemkin
then
echo "$regr_test Passed Observable Testing"
echo "<summary>$regr_test Passed Observable Testing ✅</summary>"
else
echo "$regr_test Failed Observable Testing" | tee -a $GITHUB_STEP_SUMMARY
echo "<summary>$regr_test Failed Observable Testing ❌</summary>"
export FAILED=Yes
fi
echo "</details>"
fi
echo ""
done
if [[ ${FAILED} ]]; then
echo "One or more regression tests failed." | tee -a $GITHUB_STEP_SUMMARY
echo "Please download the failed results and run the tests locally or check the above log to see why." | tee -a $GITHUB_STEP_SUMMARY
echo "⚠️ One or more regression tests failed." | tee -a $GITHUB_STEP_SUMMARY >&2
echo "Please download the failed results and run the tests locally or check the log to see why." | tee -a $GITHUB_STEP_SUMMARY >&2
fi
- name: Prepare Results for PR Comment
if : ${{ github.event_name == 'pull_request' }}
run: |
echo "<details>" > summary.txt
echo "<summary>Regression Testing Results</summary>" >> summary.txt
echo "" >> summary.txt
tail -n +1 test/regression-diff/* >> summary.txt
echo ""
echo $PR_NUMBER > summary.txt
echo "## Regression Testing Results" >> summary.txt
cat regression.stderr >> summary.txt
echo "<details>" >> summary.txt
echo "<summary>Detailed regression test results.</summary>" >> summary.txt
cat regression.stdout >> summary.txt
echo "</details>" >> summary.txt
echo "" >> summary.txt
echo "_beep boop this action was performed by a bot_ :robot:" >> summary.txt
echo "_beep boop this comment was written by a bot_ :robot:" >> summary.txt
cat summary.txt > $GITHUB_STEP_SUMMARY
- name: Write Results to PR
uses: thollander/actions-comment-pull-request@v2
- name: Upload regression summary artifact
# the annotate workflow uses this artifact to add a comment to the PR
uses: actions/upload-artifact@v3
if : ${{ github.event_name == 'pull_request' }}
with:
filePath: summary.txt

name: regression_summary
path: summary.txt

- name: Upload Comparison Results
uses: actions/upload-artifact@v3
with:
name: regression_test_comparison_results
path: |
test/regression-diff
# Install and Call codecov only if the tests were successful (permitting failures in the regression comparison tests)
- name: Code coverage install and run
if: success() || ( failure() && steps.regression-execution.conclusion == 'success' )
Expand All @@ -370,7 +386,7 @@ jobs:
# technically we could live without the 'needs' since _in theory_
# nothing will ever be merged into main that fails the tests, but
# who knows ¯\_(ツ)_/¯
#
#
# taken from https://github.com/docker/build-push-action
needs: build-and-test-linux
runs-on: ubuntu-latest
Expand Down
62 changes: 62 additions & 0 deletions .github/workflows/annotate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
name: Annotate pull request with regression summaries
# Takes a regression summary from a workflow run and annotates the pull request with it
# The pull request number is taken from the first line of the summary.txt file.
# The summary file is in an artifact called regression_summary associated
# with the workflow run that triggered this workflow.
# based on https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#using-data-from-the-triggering-workflow

on:
workflow_run:
workflows:
- Continuous Integration
types:
- completed

jobs:
on-success:
runs-on: ubuntu-latest
if: ${{ github.event.workflow_run.conclusion == 'success' }}
steps:
- run: echo 'The triggering workflow passed'
- name: 'Download regression_summary artifact'
uses: actions/github-script@v6
with:
script: |
let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: context.payload.workflow_run.id,
});
let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
return artifact.name == "regression_summary"
})[0];
let download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
let fs = require('fs');
fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/regression_summary.zip`, Buffer.from(download.data));
- name: 'Unzip artifact'
run: unzip regression_summary.zip

- name: 'Get pull request number'
id: pr_number
run: |
echo "PR_NUMBER=$( head -n 1 summary.txt )" >> $GITHUB_ENV
# remove the first line from the file
sed -i '1d' summary.txt
- name: Write summary to pull request as a comment
uses: thollander/actions-comment-pull-request@v2
with:
filePath: summary.txt
pr_number: ${{ env.PR_NUMBER }}

on-failure:
runs-on: ubuntu-latest
if: ${{ github.event.workflow_run.conclusion == 'failure' }}
steps:
- run: echo 'The triggering workflow failed'

8 changes: 4 additions & 4 deletions scripts/checkModels.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,16 @@ def checkModel(commonSpecies, uniqueSpeciesTest, uniqueSpeciesOrig, commonReacti
orig_model_species = len(commonSpecies) + len(uniqueSpeciesOrig)

logger.error(f"Original model has {orig_model_species} species.")
logger.error(f"Test model has {test_model_species} species." +
'✅' if test_model_species == orig_model_species else '❌')
logger.error(f"Test model has {test_model_species} species. " +
('✅' if test_model_species == orig_model_species else '❌'))


test_model_rxns = len(commonReactions) + len(uniqueReactionsTest)
orig_model_rxns = len(commonReactions) + len(uniqueReactionsOrig)

logger.error(f"Original model has {orig_model_rxns} reactions.")
logger.error(f"Test model has {test_model_rxns} reactions." +
'✅' if test_model_rxns == orig_model_rxns else '❌')
logger.error(f"Test model has {test_model_rxns} reactions. " +
('✅' if test_model_rxns == orig_model_rxns else '❌'))


return (test_model_species != orig_model_species) or (test_model_rxns != orig_model_rxns)
Expand Down

0 comments on commit 4ee1e71

Please sign in to comment.