Skip to content
Snippets Groups Projects
Commit 8fb7e709 authored by Christopher Wipf's avatar Christopher Wipf
Browse files

Merge branch 'test-approval-simplify' into 'master' 

simplify testing by removing unnecessary reference

See merge request !83
parents ac436d7e e1fb19b1
No related branches found
No related tags found
1 merge request!83simplify testing by removing unnecessary reference
Pipeline #121687 passed
Stage: dist
Stage: test
Stage: deploy
Hide whitespace changes
Inline Side-by-side
.gitlab-ci.yml
+ 32
− 64
View file @ 8fb7e709
......@@ -2,7 +2,6 @@ stages:
- dist
- test
- review
- docs
- deploy
# have to specify this so that all jobs execute for all commits
......@@ -28,44 +27,42 @@ gwinc/base:
cat <<EOF > Dockerfile
FROM igwn/base:buster
RUN apt-get update -qq
RUN apt-get -y install --no-install-recommends git gitlab-cli python3 python3-yaml python3-scipy python3-matplotlib python3-ipython lalsimulation-python3 python3-pypdf2 python3-h5py
RUN apt-get -y install --no-install-recommends git python3-gitlab python3 python3-yaml python3-scipy python3-matplotlib python3-ipython python3-lalsimulation python3-pypdf2 python3-h5py
RUN git clone https://gitlab-ci-token:ci_token@git.ligo.org/gwinc/inspiral_range.git
EOF
- docker build -t $IMAGE_TAG .
- docker push $IMAGE_TAG
# validate that the noises haven't changed relative to the reference
# (the reference itself could have been updated, though, see
# check_approval job)
validate_noise:
# create plots for the canonical IFOs
generate_budgets:
stage: test
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- rm -f gwinc_test_report.pdf
- export MPLBACKEND=agg
- python3 -m gwinc.test -r gwinc_test_report.pdf
- mkdir -p ifo
- export PYTHONPATH=/inspiral_range
- for ifo in $(python3 -c "import gwinc; print(' '.join(gwinc.IFOS))"); do
- python3 -m gwinc $ifo -s ifo/$ifo.png
- python3 -m gwinc $ifo -s ifo/$ifo.h5
- done
- python3 -m gwinc.ifo -s ifo/all_compare.png
artifacts:
when: on_failure
when: always
paths:
- gwinc_test_report.pdf
expose_as: 'noise validation failure report'
- ifo
# this is a special job intended to run only for merge requests where
# the test reference hash file has been updated, indicating that there
# has been a noise change. if the merge request has not yet been
# approved, generate a report of noise changes relative to the target
# branch and present that to the reviewers. if the merge request is
# approved, re-run this job, which will succeed if the MR is approved.
check_approval:
# this is a special job intended to run only for merge requests.
# budgets are compared against those from the target branch. if the
# merge request has not yet been approved and noise changes are found,
# the job will fail. once the merge request is approved the job can
# be re-run, at which point the pipeline should succeed allowing the
# merge to be merged.
noise_change_approval:
stage: review
rules:
# - if: '$CI_MERGE_REQUEST_TARGET_BRANCH_NAME == "master"'
- if: $CI_MERGE_REQUEST_ID
changes:
- gwinc/test/ref_hash
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- echo "NOISE REFERENCE CHANGE, checking approval..."
- |
cat <<EOF > check_approved.py
import sys
......@@ -85,42 +82,31 @@ check_approval:
- if [[ $approved != True ]] ; then
- old_hash=$(git cat-file -p origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME:gwinc/test/ref_hash)
- if ! python3 -m gwinc.test --git-ref $old_hash -r gwinc_test_report.pdf ; then
- echo "APPROVAL REQUIRED TO MERGE THIS BRANCH."
- echo "NOISE CHANGES RELATIVE TO $CI_MERGE_REQUEST_TARGET_BRANCH_NAME."
- echo "Approval required to merge this branch."
- /bin/false
- else
- echo "Reference update did not cause appreciable noise change."
- echo "No noise changes detected."
- fi
- else
- echo "Merge request approved, reference change accepted."
- echo "Merge request approved, noise change accepted."
- fi
artifacts:
when: on_failure
paths:
- gwinc_test_report.pdf
expose_as: 'noise changes relative to target branch head APPROVAL REQUIRED TO MERGE'
expose_as: 'noise changes relative to target branch'
# create plots for the canonical IFOs
ifo:
stage: docs
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- mkdir -p ifo
- export PYTHONPATH=/inspiral_range
- for ifo in $(python3 -c "import gwinc; print(' '.join(gwinc.IFOS))"); do
- python3 -m gwinc $ifo -s ifo/$ifo.png
- python3 -m gwinc $ifo -s ifo/$ifo.h5
- done
- python3 -m gwinc.ifo -s ifo/all_compare.png
artifacts:
when: always
paths:
- ifo
# generate the html doc web pages
html:
stage: docs
# generate the html doc web pages. the "pages" job has special
# meaning, as it's "public" artifact becomes the directory served
# through gitlab static pages
pages:
stage: deploy
only:
- master
needs:
- job: generate_budgets
artifacts: true
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- rm -rf public
......@@ -129,24 +115,6 @@ html:
- make html
- cd ..
- mv ./build/sphinx/html public
artifacts:
when: always
paths:
- public
# the "pages" job has special meaning, as it's "public" artifact
# becomes the directory served through gitlab static pages
pages:
stage: deploy
only:
- master
needs:
- job: ifo
artifacts: true
- job: html
artifacts: true
image: $CI_REGISTRY_IMAGE/gwinc/base:$CI_COMMIT_REF_NAME
script:
- mv ifo public/
artifacts:
when: always
......
CONTRIBUTING.md
+ 18
− 35
View file @ 8fb7e709
......@@ -16,11 +16,9 @@ as possible, and provide a complete, descriptive log of the changes
legibility.
`pygwinc` comes with a validation command that can compare budgets
between different versions of the code and produce a graphical report.
`pygwinc` stores a reference to the git commit that is considered the
latest good reference for the budget calculation functions, and the
validation is run against that reference by default. The command can
be run with:
from the current code against those produced from different versions
in git (by default it compares against the current HEAD). The command
can be run with:
```shell
$ python3 -m gwinc.test
```
......@@ -30,37 +28,22 @@ arbitrary commit using the '--git-ref' option. Traces for referenced
commits are cached, which speeds up subsequent comparison runs
significantly.
Commits for code changes that modify budget calculations should be
followed up updates to test reference. This can be done with the
'--update-ref' option to the test command:
```shell
$ python3 -m gwinc.test --update-ref
```
If no specific reference is provided, a pointer to the last commit
will be made. *Note: reference updates need to be made in a separate
commit after the commits that change the noise calculations* (git
commits can't have foreknowledge of their own commit hash).
Once you submit your merge request, an automated pipeline will run the
test command to validate your changes. If there are budgets
differences but the reference has not been updated, the pipeline will
fail and indicate that a reference update is required. Updates to the
reference will also cause a validation pipeline failure, but these
failures can be resolved through reviewer approval of the changes (see
below).
Once you submit your merge request a special CI job will determine if
there are budgets differences between your code and the master branch.
If there are, explicit approval from reviewers will be required before
your changes can be merged (see "approving noise" below).
## For admins: approving noise curve changes
## For reviewers: approving noise curve changes
As discussed above, merge requests that generate noise changes will
cause pipeline failures. If the MR properly includes a reference
update, then the failure should only be in the approval check. A
report will be generated comparing all noise changes against the
target branch (usually 'master'). See the 'View exposed artifacts'
menu item in the pipeline report. Once you have reviewed the report
and the code, and understand and accept the noise changes, click the
'Approve' button in the MR. Once sufficient approval has been given,
re-run the `review:check_approval` pipeline job, which should now pick
up that approval has been given and allow the pipeline to succeed.
Once the pipeline succeeds the merge can be enacted. Click the
'Merge' button to finally merge the code.
cause a pipeline failure in the `review:noise_change_approval` CI job.
The job will generate a report comparing the new noise traces against
those from master, which can be found under the 'View exposed
artifacts' menu item in the pipeline report. Once you have reviewed
the report and the code, and understand and accept the noise changes,
click the 'Approve' button in the MR. Once sufficient approval has
been given, `review:noise_change_approval` job can be re-run, which
should now pick up that approval has been given and allow the pipeline
to succeed. Once the pipeline succeeds the merge request can be
merged. Click the 'Merge' button to finally merge the code.
gwinc/test/__main__.py
+ 9
− 65
View file @ 8fb7e709
......@@ -42,28 +42,10 @@ def git_ref_resolve_hash(git_ref):
return subprocess.run(
['git', 'show', '-s', '--format=format:%H', git_ref],
capture_output=True, universal_newlines=True,
check=True,
).stdout
except subprocess.CalledProcessError:
return None
def write_ref_hash(ref_hash):
"""Write ref hash to reference file
"""
with open(test_path('ref_hash'), 'w') as f:
f.write('{}\n'.format(ref_hash))
def load_ref_hash():
"""Load the current reference git hash.
"""
try:
with open(test_path('ref_hash')) as f:
return f.read().strip()
except IOError:
return None
except subprocess.CalledProcessError as e:
logging.error(e.stderr.split('\n')[0])
def prune_cache_dir():
......@@ -79,7 +61,7 @@ def prune_cache_dir():
)[CACHE_LIMIT:]
if not expired_paths:
return
logging.info("pruning {} old caches...".format(len(expired_paths)))
logging.info("pruning {} old cache...".format(len(expired_paths)))
for path in expired_paths:
logging.debug("pruning {}...".format(path))
shutil.rmtree(path)
......@@ -248,12 +230,9 @@ def main():
'--skip', '-k', metavar='NOISE', action='append',
help='traces to skip in comparison (multiple may be specified)')
parser.add_argument(
'--git-ref', '-g', metavar='HASH',
'--git-ref', '-g', metavar='HASH', default='HEAD',
help='specify git ref to compare against')
rgroup = parser.add_mutually_exclusive_group()
rgroup.add_argument(
'--update-ref', '-u', metavar='HASH', nargs='?', const='HEAD',
help="update the stored reference git hash to HASH (or 'HEAD' if not specified) and exit")
rgroup.add_argument(
'--plot', '-p', action='store_true',
help='plot differences')
......@@ -265,46 +244,11 @@ def main():
help='specific ifos to test (default all)')
args = parser.parse_args()
# get the current hash of HEAD
head_hash = git_ref_resolve_hash('HEAD')
if not head_hash:
logging.warning("could not determine git HEAD hash.")
# update the reference if specified
if args.update_ref:
if args.update_ref == 'HEAD':
if not head_hash:
sys.exit("Could not update reference to head.")
logging.info("updating reference to HEAD...")
ref_hash = head_hash
else:
ref_hash = git_ref_resolve_hash(args.update_ref)
logging.info("updating reference git hash: {}".format(ref_hash))
write_ref_hash(ref_hash)
sys.exit()
# get the reference hash
if args.git_ref:
ref_hash = git_ref_resolve_hash(args.git_ref)
else:
ref_hash = load_ref_hash()
if not ref_hash:
pass
try:
with open(test_path('ref_hash')) as f:
ref_hash = f.read().strip()
except IOError:
logging.warning("could not open reference")
sys.exit("Unspecified reference git hash, could not run test.")
logging.info("head hash: {}".format(head_hash))
logging.info("ref hash: {}".format(ref_hash))
# don't bother test if hashes match
if ref_hash == head_hash:
logging.info("HEAD matches reference, not bothering to calculate.")
logging.info("Use --git-ref to compare against an arbitrary git commit.")
sys.exit()
ref_hash = git_ref_resolve_hash(args.git_ref)
if not ref_hash:
sys.exit("Could not resolve reference, could not run test.")
logging.info("ref hash: {}".format(ref_hash))
# load the cache
cache_path = test_path('cache', ref_hash)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment