Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Upload code coverage reports from different jobs, other CI improvements #5257

Merged
merged 28 commits into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 0 additions & 66 deletions .bulldozer.yml

This file was deleted.

196 changes: 160 additions & 36 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,115 @@ jobs:
# file with notes on your contribution.
git diff --name-only $(git merge-base origin/main HEAD) | grep '^CHANGELOG.md$' && echo "Thanks for helping keep our CHANGELOG up-to-date!"

style:
name: Style
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2

- name: Setup Python
uses: actions/setup-python@v1
with:
python-version: 3.8

- name: Install requirements
run: |
grep -E '^black' dev-requirements.txt | xargs pip install

- name: Debug info
run: |
pip freeze

- name: Run black
if: '! cancelled()'
Copy link
Member

Choose a reason for hiding this comment

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

Why is this check necessary? Doesn't it always cancel all the checks?

Copy link
Member Author

Choose a reason for hiding this comment

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

This can be removed, it was only need when this check was in the other workflow that had a lot of different steps. And then it survived the copy-paste.

run: |
black --check .

lint:
name: Lint
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2

- name: Setup Python
uses: actions/setup-python@v1
with:
python-version: 3.8

- uses: actions/cache@v2
with:
path: ${{ env.pythonLocation }}
key: ${{ runner.os }}-pydeps-${{ env.pythonLocation }}-${{ hashFiles('setup.py') }}-${{ hashFiles('dev-requirements.txt') }}

- name: Install requirements
run: |
make install

- name: Debug info
run: |
pip freeze

- name: Run flake8
if: '! cancelled()'
run: |
flake8 .

- name: Run mypy
if: '! cancelled()'
run: |
mypy . --cache-dir=/dev/null

gpu_checks:
name: GPU Checks
if: github.repository == 'allenai/allennlp' # self-hosted runner only available on main repo
timeout-minutes: 15
runs-on: [self-hosted, GPU]
env:
# Required to use the setup-python action.
AGENT_TOOLSDIRECTORY: '/opt/hostedtoolcache'
# Our self-hosted runner currently is currently compatible with CUDA 11.*.
TORCH_VERSION: 'torch==1.8.1+cu111 torchvision==0.9.1+cu111 -f https://download.pytorch.org/whl/torch_stable.html'

steps:
- uses: actions/checkout@v2

- name: Set Docker tag
run: |
if [[ $GITHUB_EVENT_NAME == 'release' ]]; then
echo "DOCKER_TAG=${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV;
else
echo "DOCKER_TAG=$GITHUB_SHA" >> $GITHUB_ENV;
fi
- name: Setup Python
uses: actions/setup-python@v2
env:
ACTIONS_STEP_DEBUG: 'true'
with:
python-version: 3.8

- name: Build test image
- uses: actions/cache@v2
with:
path: ${{ env.pythonLocation }}
key: ${{ runner.os }}-pydeps-${{ env.pythonLocation }}-${{ env.TORCH_VERSION }}-${{ hashFiles('setup.py') }}-${{ hashFiles('dev-requirements.txt') }}

- name: Install requirements
run: |
make docker-test-image DOCKER_TAG=$DOCKER_TAG
make install

- name: Run GPU tests
run: |
make docker-test-run DOCKER_TAG=$DOCKER_TAG ARGS='gpu-test'
make gpu-test
mkdir coverage
mv coverage.xml coverage/

- name: Save coverage report
if: github.repository == 'allenai/allennlp' && (github.event_name == 'push' || github.event_name == 'pull_request')
uses: actions/upload-artifact@v1
with:
name: gpu-tests-coverage
path: ./coverage

- name: Clean up
if: always()
run: |
# Could run into issues with the cache if we don't uninstall the editable.
# See https://github.com/pypa/pip/issues/4537.
pip uninstall --yes allennlp
Copy link
Member

Choose a reason for hiding this comment

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

This reads like some hard-won wisdom.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed.


check_core:
name: Check Core
Expand Down Expand Up @@ -84,36 +169,19 @@ jobs:
run: |
pip freeze

- name: Format
if: '! cancelled()'
run: |
make format

- name: Lint
if: '! cancelled()'
run: |
make lint

- name: Type check
if: '! cancelled()'
run: |
make typecheck

- name: Run tests
if: '! cancelled()'
run: |
make test-with-cov
make test
mkdir coverage
mv coverage.xml coverage/

- name: Upload coverage to Codecov
- name: Save coverage report
if: matrix.python == '3.7' && github.repository == 'allenai/allennlp' && (github.event_name == 'push' || github.event_name == 'pull_request')
uses: codecov/codecov-action@v1
uses: actions/upload-artifact@v1
with:
file: ./coverage.xml
# Ignore codecov failures as the codecov server is not
# very reliable but we don't want to report a failure
# in the github UI just because the coverage report failed to
# be published.
fail_ci_if_error: false
name: cpu-tests-coverage
path: ./coverage

- name: Clean up
if: always()
Expand Down Expand Up @@ -160,13 +228,69 @@ jobs:

- name: Run models tests
run: |
cd allennlp-models && make test
cd allennlp-models
make test-with-cov COV=allennlp
mkdir coverage
mv coverage.xml coverage/

- name: Save coverage report
if: matrix.python == '3.7' && github.repository == 'allenai/allennlp' && (github.event_name == 'push' || github.event_name == 'pull_request')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we fix 3.7 here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, this was needed when these tests ran on multiple versions of Python, because we only needed one coverage report from this job.

uses: actions/upload-artifact@v1
with:
name: model-tests-coverage
path: allennlp-models/coverage

- name: Clean up
if: always()
run: |
pip uninstall --yes allennlp allennlp-models

upload_coverage:
name: Upload Coverage Report
if: github.repository == 'allenai/allennlp' && (github.event_name == 'push' || github.event_name == 'pull_request')
runs-on: ubuntu-latest
# needs: [check_core, gpu_checks]
needs: [check_core, check_models, gpu_checks]

steps:
# Need to checkout code to get the coverage config.
- uses: actions/checkout@v2

- name: Download coverage report from CPU tests
uses: actions/download-artifact@v1
with:
name: cpu-tests-coverage
path: coverage/cpu_tests

- name: Download coverage report from GPU Tests
uses: actions/download-artifact@v1
with:
name: gpu-tests-coverage
path: coverage/gpu_tests

- name: Download coverage report from model tests
uses: actions/download-artifact@v1
with:
name: model-tests-coverage
path: coverage/model_tests

- name: Debug info
run: |
ls -lh coverage
ls -lh coverage/cpu_tests
ls -lh coverage/gpu_tests
ls -lh coverage/model_tests
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not, I'll take out.


- name: Upload coverage to Codecov
uses: codecov/codecov-action@v1
with:
files: coverage/cpu_tests/coverage.xml,coverage/gpu_tests/coverage.xml,coverage/model_tests/coverage.xml
# Ignore codecov failures as the codecov server is not
# very reliable but we don't want to report a failure
# in the github UI just because the coverage report failed to
# be published.
fail_ci_if_error: false

# Builds package distribution files for PyPI.
build_package:
name: Build Package
Expand Down Expand Up @@ -481,7 +605,7 @@ jobs:
# Publish the core distribution files to PyPI.
publish:
name: PyPI
needs: [check_core, check_models, gpu_checks, build_package, test_package, docker, docs]
needs: [style, lint, check_core, check_models, gpu_checks, build_package, test_package, docker, docs]
# Only publish to PyPI on releases and nightly builds to "allenai/allennlp" (not forks).
if: github.repository == 'allenai/allennlp' && (github.event_name == 'release' || github.event_name == 'schedule')
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ When you're ready to contribute code to address an open issue, please follow the

Our CI also uses [`flake8`](https://github.com/allenai/allennlp/tree/main/tests) to lint the code base and [`mypy`](http://mypy-lang.org/) for type-checking. You should run both of these next with

make lint
flake8 .

and

Expand Down
16 changes: 10 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ MD_DOCS_CONF_SRC = mkdocs-skeleton.yml
MD_DOCS_TGT = site/
MD_DOCS_EXTRAS = $(addprefix $(MD_DOCS_ROOT),README.md CHANGELOG.md CONTRIBUTING.md)

TORCH_VERSION = torch==1.8.1 torchvision==0.9.1

DOCKER_TAG = latest
DOCKER_IMAGE_NAME = allennlp/allennlp:$(DOCKER_TAG)
DOCKER_TEST_IMAGE_NAME = allennlp/test:$(DOCKER_TAG)
Expand Down Expand Up @@ -57,18 +59,18 @@ typecheck :

.PHONY : test
test :
pytest --color=yes -rf --durations=40

.PHONY : test-with-cov
test-with-cov :
pytest --color=yes -rf --durations=40 \
pytest --color=yes -v -rf --durations=40 \
--cov-config=.coveragerc \
--cov=$(SRC) \
--cov-report=xml

.PHONY : gpu-test
gpu-test : check-for-cuda
pytest --color=yes -v -rf -m gpu
pytest --color=yes -v -rf --durations=20 \
--cov-config=.coveragerc \
--cov=$(SRC) \
--cov-report=xml \
-m gpu

.PHONY : benchmarks
benchmarks :
Expand All @@ -85,6 +87,8 @@ install :
# Due to a weird thing with pip, we may need egg-info before running `pip install -e`.
# See https://github.com/pypa/pip/issues/4537.
python setup.py install_egg_info
# Install torch ecosystem first.
pip install $(TORCH_VERSION)
pip install --upgrade --upgrade-strategy eager -e . -r dev-requirements.txt
# These nltk packages are used by the 'checklist' module.
python -c 'import nltk; [nltk.download(p) for p in ("wordnet", "wordnet_ic", "sentiwordnet")]'
Expand Down
11 changes: 9 additions & 2 deletions allennlp/common/testing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@
from allennlp.training.metrics import Metric


_available_devices = ["cpu"] + (["cuda:0"] if torch.cuda.is_available() else [])
_available_devices = ["cuda:0"] if torch.cuda.is_available() else ["cpu"]


def multi_device(test_method):
"""
Decorator that provides an argument `device` of type `str` for each available PyTorch device.
Decorator that provides an argument `device` of type `str` to a test function.

If you have a CUDA capable GPU available, device will be "cuda:0", other the device will
epwalsh marked this conversation as resolved.
Show resolved Hide resolved
be "cpu".

!!! Note
If you have a CUDA capable GPU available, but you want to run the test using CPU only,
just set the environment variable "CUDA_CAPABLE_DEVICES=''" before running pytest.
"""
return pytest.mark.parametrize("device", _available_devices)(pytest.mark.gpu(test_method))

Expand Down