replace Travis CI with GitHub actions by orbeckst · Pull Request #52 · MDAnalysis/MDAnalysisData

orbeckst · 2021-10-01T17:40:58Z

codecov · 2021-10-01T17:41:43Z

Codecov Report

Merging #52 (1631093) into master (4cb391f) will increase coverage by 0.11%.
The diff coverage is n/a.

❗ Current head 1631093 differs from pull request most recent head 4572c0c. Consider uploading reports for the commit 4572c0c to get more accurate results

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
+ Coverage   56.53%   56.64%   +0.11%     
==========================================
  Files          12       13       +1     
  Lines         375      376       +1     
  Branches       47       47              
==========================================
+ Hits          212      213       +1     
  Misses        160      160              
  Partials        3        3

Impacted Files	Coverage Δ
MDAnalysisData/authors.py	`100.00% <0.00%> (ø)`

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 4cb391f...4572c0c. Read the comment docs.

pep8speaks · 2021-10-01T19:53:36Z

Hello @orbeckst! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-05 00:37:49 UTC

orbeckst · 2021-10-01T20:47:38Z

I am not sure why coverage looks so abysmal. Otherwise this seems to work.

orbeckst · 2021-10-01T20:53:32Z

The docs build action will only run when it is merged into master.

IAlibay

Couple of comments but nothing blocking so I'll approve.

IAlibay · 2021-10-02T14:50:41Z

+        os: [ubuntu-latest, macOS-latest, windows-latest]
+        python-version: [2.7, 3.6, 3.7, 3.8, 3.9]
+        exclude:
+          - os: macOS-latest


the exclude here is weird, I guess you're only testing 3.8 and 2.7 on macOS and Windows? It might be a bit simpler to just not have these in the main matrix and just add them as includes (4 entries instead of 6 in exclude)

good call, changing it

IAlibay · 2021-10-02T14:51:53Z

+        python -m pip install wheel
+        python -m pip install pytest pytest-cov pytest-pep8 pytest-mock codecov
+
+    - name: Install main dependencies


any reason for having the two separate parts of the workflow and not just squashing everything into the one step?

Mainly because I like the semantics of clearly seeing where something for tests only is done vs what's needed for running the package — I often look at the CI files to figure out what's actually needed as opposed to what someone might or might not have put into setup.py.

- Tests only (gratuitously expanded to MacOS and Windows), gh-ci.yml - add docs action - install pathlib2 for 2.7 - updated supported Python versions in setup.py (2.7, 3.6-3.9)

- use pathlib for OS-agnostic path comparisons - install pathlib2 in setup.py for Python 2.7

Ensure that CRLF/LF line endings do not matter for tests. Fixed sha256 test, which incorrectly wrote a text file instead of binary.

orbeckst · 2021-10-05T00:16:20Z

Coverage is getting annoying on GitHub actions: I got the error

Coverage.py warning: No data was collected. (no-data-collected)

and following https://stackoverflow.com/a/60579142 I added tests/__init__.py but now the generated authors file is not correctly picked up by the tests:

 _________________________________ test_authors _________________________________

    def test_authors():
        authors = MDAnalysisData.__authors__
>       assert authors[:5] == ["Oliver Beckstein", "Shujie Fan",
                               "Richard J. Gowers", "Micaela Matta",
                               "Lily Wang"]
E       AssertionError: assert ['The MDAnaly...lopment Team'] == ['Oliver Beck..., 'Lily Wang']
E         At index 0 diff: 'The MDAnalysis Development Team' != 'Oliver Beckstein'
E         Right contains 4 more items, first extra item: 'Shujie Fan'
E         Full diff:
E           [
E         +  'The MDAnalysis Development Team',
E         -  'Oliver Beckstein',
E         -  'Shujie Fan',
E         -  'Richard J. Gowers',
E         -  'Micaela Matta',
E         -  'Lily Wang',
E           ]

MDAnalysisData/tests/test_package_metadata.py:13: AssertionError
=============================== warnings summary ===============================
MDAnalysisData/__init__.py:20
  /home/runner/work/MDAnalysisData/MDAnalysisData/MDAnalysisData/__init__.py:20: UserWarning: Could not find authors.py, __authors__ will be the generic MDAnalysis team.
    warnings.warn('Could not find authors.py, __authors__ will be the '

I am not sure if it's trying to run the package from the sources instead from the proper installation.

orbeckst · 2021-10-05T00:23:25Z

The author tests work just fine if there is no tests/__init__.py.

What exactly do pytest and coverage look for and where???

orbeckst · 2021-10-05T00:34:54Z

The only way I managed to solve the issue of getting both pytest and coverage to work was to use an in-place installation pip install -e ..

(Seems a sub-par solution but for right now I take it and I am eager to hear better solutions and will happily implement them.)

- add tests/__init__.py to coax pytest-cov to report coverage "CoverageWarning: No data was collected. (no-data-collected)" is supposedly fixed with this empty __init__.py (see https://stackoverflow.com/a/60579142) - inplace-installation so that coverage and pytest work with generated authors.py (with standard installation AND the tests/__init__.py, pytest picks up the local sources that does not contain the generated authors.py and fails)

orbeckst · 2021-10-05T00:40:36Z

https://stackoverflow.com/questions/20292950/making-py-test-coverage-and-tox-work-together-init-py-in-tests-folder seems to have some links for follow-up reading.

orbeckst force-pushed the travis2ghactions branch from 6f4e45b to c876c08 Compare October 1, 2021 20:30

orbeckst requested review from IAlibay and tylerjereddy October 1, 2021 20:47

orbeckst mentioned this pull request Oct 1, 2021

Make next release 3.6+? #51

Closed

tylerjereddy added the CI label Oct 1, 2021

IAlibay approved these changes Oct 2, 2021

View reviewed changes

orbeckst force-pushed the travis2ghactions branch from ee61e44 to f801946 Compare October 4, 2021 23:30

orbeckst added 5 commits October 4, 2021 17:02

replace Travis CI with GitHub actions

01387e6

- Tests only (gratuitously expanded to MacOS and Windows), gh-ci.yml - add docs action - install pathlib2 for 2.7 - updated supported Python versions in setup.py (2.7, 3.6-3.9)

fix version test for Python 2.7

3a50dbf

fix tests for paths (UNIX/Windows)

359d60d

- use pathlib for OS-agnostic path comparisons - install pathlib2 in setup.py for Python 2.7

fixed tests with line endings for UNIX/Windows

386c100

Ensure that CRLF/LF line endings do not matter for tests. Fixed sha256 test, which incorrectly wrote a text file instead of binary.

updated README badge for GH and added Contributing section

6847bc0

orbeckst force-pushed the travis2ghactions branch from f801946 to a092e30 Compare October 5, 2021 00:03

orbeckst force-pushed the travis2ghactions branch from 1631093 to 4572c0c Compare October 5, 2021 00:37

orbeckst merged commit 779bed1 into master Oct 5, 2021

orbeckst deleted the travis2ghactions branch October 5, 2021 00:40

orbeckst mentioned this pull request Oct 5, 2021

version information in docs is missing #54

Closed

Conversation

orbeckst commented Oct 1, 2021

Uh oh!

codecov Bot commented Oct 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pep8speaks commented Oct 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-10-05 00:37:49 UTC

Uh oh!

orbeckst commented Oct 1, 2021

Uh oh!

orbeckst commented Oct 1, 2021

Uh oh!

IAlibay left a comment

Choose a reason for hiding this comment

Uh oh!

IAlibay Oct 2, 2021

Choose a reason for hiding this comment

Uh oh!

orbeckst Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

IAlibay Oct 2, 2021

Choose a reason for hiding this comment

Uh oh!

orbeckst Oct 4, 2021

Choose a reason for hiding this comment

Uh oh!

orbeckst commented Oct 5, 2021

Uh oh!

orbeckst commented Oct 5, 2021

Uh oh!

orbeckst commented Oct 5, 2021

Uh oh!

orbeckst commented Oct 5, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Oct 1, 2021 •

edited

Loading

pep8speaks commented Oct 1, 2021 •

edited

Loading