Tox + code coverage #150

algattik · 2020-01-24T22:22:01Z

Changes:

Allow developers to lint & test locally in a single command: streamlined use of flake8 and pytest within tox.

$ tox
python run-test-pre: PYTHONHASHSEED='1159078597'
python run-test: commands[0] | flake8 --output-file=lint-testresults.xml --format junit-xml
python run-test: commands[1] | pytest tests/unit --cov=code --cov-report=xml --junitxml=unit-testresults.xml
=========================================================================== test session starts ===========================================================================
...
tests/unit/code_test.py .                                                                                                                                           [ 20%]
tests/unit/data_test.py .
...                                                                                                                                        
---------- coverage: platform darwin, python 3.7.6-final-0 -----------
Coverage HTML written to dir htmlcov
Coverage XML written to file /Users/algattik/GitHub/MLOpsPython2/coverage.xml

====================================================================== 5 passed, 4 warnings in 6.34s ======================================================================
_________________________________________________________________________________ summary _________________________________________________________________________________
  python: commands succeeded
  congratulations :)

Added code coverage report. Required adapted pytest commandline (now in tox.ini) and installing .NET Core as for security reasons, the AzDO PublishCodeCoverageResults does not publish user-generated HTML reports anymore.
Linting fixes in some .py scripts to avoid CI build failing.
Some simplifications in the build pipeline YAML.

bump pip requirements versions (microsoft#104)

commit 3ba351c Author: Alexandre Gattiker <algattik@users.noreply.github.com> Date: Sat Jan 25 08:37:53 2020 +0100 . commit 5c3c3e0 Author: Alexandre Gattiker <algattik@users.noreply.github.com> Date: Sat Jan 25 07:58:50 2020 +0100 . commit 2a49221 Author: Alexandre Gattiker <algattik@users.noreply.github.com> Date: Sat Jan 25 07:24:37 2020 +0100 . Merge remote-tracking branch 'upstream/master' into algattik/code-coverage

eedorenko · 2020-01-27T23:23:29Z

docs/development_setup.md

+Install the required Python modules in your virtual environment.
+
+```
+pip install -r environment_setup/build-image/requirements.txt 


requirements.txt is in environment_setup folder

It should be "pip install -r environment_setup/requirements.txt"

Fixed, apologies

eedorenko · 2020-01-27T23:50:31Z

environment_setup/Dockerfile

+# Install Python modules
+COPY requirements.txt  /setup/
+RUN pip install --upgrade -r /setup/requirements.txt


Consider reducing the number of image layers by combining RUN commands.
"squash" option is still experimental.

Updated to reduce RUN steps

eedorenko · 2020-01-27T23:52:48Z

The build fails with "No code coverage results were found to publish."

algattik · 2020-01-30T05:39:20Z

The build fails with "No code coverage results were found to publish."

The mlopspython image must first be updated as there are new dependencies.
Here is a sample build (on a derived branch which only replaces the mlopspython container with a privately build container from the Dockerfile):
https://dev.azure.com/algattik/MLOpsPython/_build/results?buildId=300&view=codecoverage-tab

algattik · 2020-01-30T05:39:36Z

Updated with changes from master

dtzar · 2020-01-30T17:52:04Z

environment_setup/Dockerfile

+# Install dotnet runtime used to generate code coverage report
+RUN curl -O https://packages.microsoft.com/config/ubuntu/19.04/packages-microsoft-prod.deb \
+    && dpkg -i packages-microsoft-prod.deb \
+    && apt-get update && apt-get install -y dotnet-runtime-3.1


Do we really need dotnet-runtime-3.1? I assume code coverage will still show up in Azure DevOps, but we'll just get the warning of no auto-generated html reports, no?

Without dotnet runtime we only get the report as an artifact.

With dotnet runtime we get an an embedded HTML report

this can be achieved by:

Adding dotnet runtime to mlopsython container, is an additional 104 MB installation.

install dotnet at pipeline execution time.

Given the impact of 1 or 2 in terms of build time and the limited benefit of the embedded report, I will update the PR to go for neither option.

Another idea would be to add 2. commented out in the pipeline if people want to enable the code coverage html report

OK. Note that we will see this warning:

Fixed. There is no update to the Dockerfile anymore, but the mlopspython image must still be rebuilt because of updates to requirements.txt.
I tested installing dotnet core but coverage report generation then fails with a missing libicu library, and sudo is not available to install it. I suggest investigating this at a later point.

eedorenko

David Tesar's comment:
"Do we really need dotnet-runtime-3.1? I assume code coverage will still show up in Azure DevOps, but we'll just get the warning of no auto-generated html reports, no?"

…erage

algattik · 2020-01-31T06:25:24Z

Implemented change due to PR #162, that breaks compatibility with tox virtual environments.

dc00144: Run linter and unit tests in the system python (i.e. in conda environment). This also makes things faster as tox does not have to build a venv.

algattik · 2020-01-31T06:27:13Z

After we merge #158 we should be able to add instructions for enabling report generation:

Add icu to CI conda environment
Add dotnet core install task in Build pipeline

algattik added 4 commits November 28, 2019 18:16

Merge pull request #2 from microsoft/master

f69a2ca

bump pip requirements versions (microsoft#104)

Merge remote-tracking branch 'upstream/master'

7b370c1

.

8556d62

.

979d846

algattik mentioned this pull request Jan 24, 2020

Generate a Docker container within the CI build #142

Closed

algattik added 3 commits January 24, 2020 23:26

Update requirements.txt

08e76e3

Update azdo-ci-build-train.yml

f25590a

algattik marked this pull request as ready for review January 25, 2020 15:51

algattik requested a review from eedorenko January 25, 2020 15:51

eedorenko self-assigned this Jan 27, 2020

eedorenko suggested changes Jan 27, 2020

View reviewed changes

algattik added 4 commits January 29, 2020 20:46

Merge branch 'master' of https://github.com/microsoft/MLOpsPython

3ab3230

Merge branch 'master' into algattik/code-coverage

d29a008

Update Dockerfile

2b7707f

Update tox.ini

26e1a01

algattik requested a review from eedorenko January 30, 2020 05:39

dtzar reviewed Jan 30, 2020

View reviewed changes

Update development_setup.md

ef14bc8

eedorenko suggested changes Jan 30, 2020

View reviewed changes

algattik and others added 2 commits January 30, 2020 20:24

Reverted Dockerfile

b4268af

Merge branch 'master' into algattik/code-coverage

6596317

eedorenko approved these changes Jan 30, 2020

View reviewed changes

algattik added 2 commits January 31, 2020 06:05

Merge remote-tracking branch 'upstream/master' into algattik/code-cov…

d7308fc

…erage

Do not use tox virtualenv

dc00144

eedorenko merged commit e0e6d22 into microsoft:master Jan 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tox + code coverage #150

Tox + code coverage #150

algattik commented Jan 24, 2020 •

edited

Loading

eedorenko Jan 27, 2020

eedorenko Jan 30, 2020

algattik Jan 30, 2020

eedorenko Jan 27, 2020

algattik Jan 30, 2020

eedorenko commented Jan 27, 2020

algattik commented Jan 30, 2020

algattik commented Jan 30, 2020

dtzar Jan 30, 2020

algattik Jan 30, 2020

dtzar Jan 30, 2020

algattik Jan 30, 2020

dtzar Jan 30, 2020

algattik Jan 30, 2020

eedorenko left a comment

algattik commented Jan 31, 2020

algattik commented Jan 31, 2020

Tox + code coverage #150

Tox + code coverage #150

Conversation

algattik commented Jan 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eedorenko commented Jan 27, 2020

algattik commented Jan 30, 2020

algattik commented Jan 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eedorenko left a comment

Choose a reason for hiding this comment

algattik commented Jan 31, 2020

algattik commented Jan 31, 2020

algattik commented Jan 24, 2020 •

edited

Loading