Coverage & CI Setup Improvement #335

vmuriart · 2017-01-27T08:56:01Z

What does this implement/fix? Explain your changes.

Add coverage (cs code limited to windows, no mature coverage tools for mono)
Cleans up CI configurations
Speeds up Travis ~30% by using containers
Simplifies and unfreezes mono setup
Speed up AppVeyor by 50% by not downloading and building with miniconda.

Does this close any currently open issues?

#237
#334

Any other comments?

Sample coverage report.

Checklist

Check all those that are applicable and complete.

Updated the CHANGELOG

codecov-io · 2017-01-27T09:09:20Z

Codecov Report

❗ No coverage uploaded for pull request base (master@ef133a3).

@@           Coverage Diff            @@
##             master    #335   +/-   ##
========================================
  Coverage          ?   61.1%           
========================================
  Files             ?      61           
  Lines             ?    5324           
  Branches          ?     897           
========================================
  Hits              ?    3253           
  Misses            ?    1832           
  Partials          ?     239

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef133a3...44ba698. Read the comment docs.

den-run-ai · 2017-01-29T06:03:30Z

.travis.yml

-  - sudo DEBIAN_FRONTEND=noninteractive apt-get -y -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confnew" install mono-devel mono-complete referenceassemblies-pcl ca-certificates-mono nunit-console
+env:
+  global:
+    - LD_PRELOAD=/lib/x86_64-linux-gnu/libSegFault.so


can you explain the purpose of this?

It can help get a bit more info on the tracebacks:
http://stackoverflow.com/a/18947029/5208670

den-run-ai · 2017-01-29T06:18:32Z

@vmuriart this pull request removes the conda packages. Is there any reason for this?

den-run-ai · 2017-01-29T06:19:16Z

@vmuriart can we break this into 2 PR? one for code coverage and one for build improvements?

vmuriart · 2017-01-29T06:21:54Z

The conda packaging is what makes the builds the AppVeyor extremely slow and we don't even use them for testing. Do we need them as part of the CI?

den-run-ai · 2017-01-29T06:29:51Z

@vmuriart we build conda packages, and provide the recipe and downloadable binaries (from appveyor artifacts) for everyone to use in this project. @filmor was very happy about conda support. we do not test conda package, but could add couple lines in appveyor.yml to do this.

Regarding the low speed of the builds - we can re-use installed Miniconda environments from appveyor CI.

It is also important to show how the conda recipe is used to build the package, since this is not trivial.

Finally I plan to extend this conda recipe to conda-forge.

vmuriart · 2017-01-29T06:34:56Z

I tried using the built in miniconda's, they were incomplete (don't have the build command 😞.

I thought the interest on the original pr was to create the recipe for miniconda (which is still with the package). @filmor can clarify a bit more on this.

If the intent though really is for AppVeyor to build the miniconda packages, we can have them build only on pr or when the build was triggered by a schedule (ie, they would be built once a day)

den-run-ai · 2017-01-29T06:42:52Z

You are correct on the original intent of the conda recipe. But if you follow the conversation, then it is obvious that without showing the actual build setup, it is extremely hard to get it right. conda build is installed like this: `conda install conda-build` I'm ok with automated nightly builds for conda (no user intervention) or with migrating the builds to conda-forge. But not removing the conda builds.

…

On Sun, Jan 29, 2017 at 12:34 AM, Victor Uriarte ***@***.***> wrote: I tried using the built in miniconda's, they were incomplete (don't have the build command 😞. I thought the interest on the original pr was to create the recipe for miniconda (which is still with the package). @filmor <https://github.com/filmor> can clarify a bit more on this. If the intent though really is for AppVeyor to build the miniconda packages, we can have them build only on pr or when the build was triggered by a schedule (ie, they would be built once a day) — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#335 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHgZ5aFbki7NAKYpO2VvWrq-winYn_hSks5rXDMRgaJpZM4Lvi59> .

vmuriart · 2017-01-29T08:41:32Z

Long-term, moving this to conda-forge is probably the better way. For now I rewrote the pull request to keep conda-build part of the CI, but only build them if its part of a Pull Request.

On my fork they weren't building, but it did once it was uploaded to this pull request.
It makes more sense to build on pull requests than on a scheduled basis, since the scheduled one will probably keep rebuilding the same unchanged branch.

vmuriart · 2017-01-29T09:06:17Z

whoops, deleted the wrong branch earlier. sorry

filmor

In general, I think we should have most if not all testing integrated into the conda build recipe. That could simplify the travis and appveyor configuration quite a bit

filmor · 2017-01-30T09:17:24Z

appveyor.yml

+    - PYTHON_VERSION: 3.3
+    - PYTHON_VERSION: 3.4
+    - PYTHON_VERSION: 3.5
+    - PYTHON_VERSION: 3.6


Are you sure about this? Wouldn't this result in the version numbers being parsed as floats?

They are still being used as strings. Remeber this is doing set PYTHON_VERSION=3.3 which comes out as a string when you do %PYTHON_VERSION%

filmor · 2017-01-30T09:18:23Z

appveyor.yml

+  - set PYTHON=C:\PYTHON%PYTHON_VERSION:.=%
+  - if %PLATFORM%==x86 (set CONDA_BLD_ARCH=32)
+  - if %PLATFORM%==x86 (set NUNIT=%NUNIT%-x86)
+  - if %PLATFORM%==x64 (set PYTHON=%PYTHON%-x64)


Why don't you use %CONDA_PY%? Why change the casing?

%CONDA_PY% was removed originally, I added it back later, but decided to keep it on PYTHON_VERSION in case it was removed later.
The casing was to keep all constants on upper case and the rest on lowercase.

filmor · 2017-01-30T09:19:14Z

appveyor.yml

+  # Shortcut path to executables. Mostly because of OpenCover
+  - set PYTHON_EXE=%PYTHON%\python.exe
+  - set NUNIT_EXE=.\packages\NUnit.Runners.2.6.2\tools\%NUNIT%.exe
+  - set OPENCOVER_EXE=.\packages\OpenCover.4.6.519\tools\OpenCover.Console.exe


You are hard-coding versions here, this just leads to unnecessary pain (e.g. I'd like to update NUnit in the future).

Its a very annoying shortcoming from OpenCover.

filmor · 2017-01-30T09:22:51Z

appveyor.yml


 test_script:
-  - ps: '& "$env:PYTHON\\Scripts\\pip.exe" install --no-cache-dir --force-reinstall --ignore-installed ("dist\\" + (gci dist\*.whl)[0].Name)'
-  - ps: copy-item (gci -path build -re -include Python.Test.dll)[0].FullName C:\testdir
-  - "%PYTHON%\\python.exe src\\tests\\runtests.py"


Is this still carried out somewhere?

vmuriart · 2017-01-30T20:28:06Z

@filmor My problem with that is that the conda build adds an extra 60-90 seconds per build and then it wouldnt be testing under our standard packing that we would use on pypi. If you unpack the wheel and the conda recipe output, they are nearly the same so double testing/building is redundant with the exception that conda build puts alot of it out of our hands.

Originally I wanted to remove the conda build altogether from the CI testing which simplifies this alot more, but was talked into leaving it and made it so that it builds once a pull_request has been submitted.

+ Unfreezes mono + Moves nunit runner to packages.config

vmuriart requested review from filmor, tonyroberts and den-run-ai January 27, 2017 08:57

vmuriart self-assigned this Jan 27, 2017

vmuriart force-pushed the ci branch from 1cfe592 to d9a4702 Compare January 29, 2017 01:43

vmuriart mentioned this pull request Jan 29, 2017

include fake c libs for pycparser to work #338

Merged

den-run-ai reviewed Jan 29, 2017

View reviewed changes

den-run-ai added the decision label Jan 29, 2017

vmuriart force-pushed the ci branch from d9a4702 to fc917b7 Compare January 29, 2017 06:37

vmuriart force-pushed the ci branch from fc917b7 to c5ca8e6 Compare January 29, 2017 08:31

vmuriart force-pushed the ci branch from d6b7404 to 44d9d85 Compare January 29, 2017 08:51

vmuriart closed this Jan 29, 2017

vmuriart deleted the ci branch January 29, 2017 08:56

vmuriart restored the ci branch January 29, 2017 08:57

vmuriart reopened this Jan 29, 2017

vmuriart mentioned this pull request Jan 29, 2017

String directives cleanup on Runtime #339

Merged

filmor reviewed Jan 30, 2017

View reviewed changes

vmuriart added 3 commits January 30, 2017 16:38

Speed up Travis - use containers

49c6143

+ Unfreezes mono + Moves nunit runner to packages.config

Build conda recipe on Pull Requests

a74fe0a

Add coverage

3e59b33

Add tox for quicker local testing

44ba698

vmuriart force-pushed the ci branch from 44d9d85 to 44ba698 Compare January 30, 2017 23:58

vmuriart closed this Jan 31, 2017

vmuriart mentioned this pull request Jan 31, 2017

Coverage & CI Setup Improvement #345

Merged

vmuriart deleted the ci branch January 31, 2017 03:20

Coverage & CI Setup Improvement #335

Coverage & CI Setup Improvement #335

Uh oh!

Conversation

vmuriart commented Jan 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

Uh oh!

codecov-io commented Jan 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

den-run-ai commented Jan 29, 2017

Uh oh!

den-run-ai commented Jan 29, 2017

Uh oh!

vmuriart commented Jan 29, 2017

Uh oh!

den-run-ai commented Jan 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmuriart commented Jan 29, 2017

Uh oh!

den-run-ai commented Jan 29, 2017 via email

Uh oh!

vmuriart commented Jan 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmuriart commented Jan 29, 2017

Uh oh!

filmor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vmuriart commented Jan 30, 2017

Uh oh!

Uh oh!

vmuriart commented Jan 27, 2017 •

edited

Loading

codecov-io commented Jan 27, 2017 •

edited

Loading

den-run-ai commented Jan 29, 2017 •

edited

Loading

vmuriart commented Jan 29, 2017 •

edited

Loading