Skip to content

Lint and test on GitHub Actions#1122

Merged
timothycrosley merged 2 commits intoPyCQA:developfrom
hugovk:add-gha
May 12, 2020
Merged

Lint and test on GitHub Actions#1122
timothycrosley merged 2 commits intoPyCQA:developfrom
hugovk:add-gha

Conversation

@hugovk
Copy link
Contributor

@hugovk hugovk commented Feb 10, 2020

For #1121.

  • Add a workflow for linting
    • Only runs on ubuntu-latest with Python 3.8
  • Add a workflow for poetry run pytest
    • Runs on ubuntu-18.04, ubuntu-16.04, macos-latest
    • Maybe a single ubuntu-latest would be enough?
    • With Python 3.6-3.8

Examples:

Not included:

For some reason, 3.8 on macOS is failing:

>           assert str(tmpdir.join("file1.py")) in out
E           AssertionError: assert '/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0/file1.py' in ''
E            +  where '/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0/file1.py' = str(local('/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0/file1.py'))
E            +    where local('/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0/file1.py') = <bound method LocalPath.join of local('/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0')>('file1.py')
E            +      where <bound method LocalPath.join of local('/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0')> = local('/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/test_command_line_True_0').join

https://github.com/hugovk/isort/runs/436539509?check_suite_focus=true#step:7:218

Any ideas?

@hugovk hugovk mentioned this pull request Feb 10, 2020
@timothycrosley
Copy link
Member

This all looks fantastic! Thank you! Similar to PyLint, I'd feel okay leaking the coverage key if needed for now. I'm not sure the cause of the mac os failure, but I'm going to work on securing a permanent mac testing machine to help diagnose.

Thanks!

~Timothy

@hugovk
Copy link
Contributor Author

hugovk commented Feb 18, 2020

You'll need to fetch the coverage token from https://codecov.io/gh/timothycrosley/isort/settings, but I can help set it up, if you like.

Would you like it included in this PR or a follow up? It might be better in a follow up, because then GHA will have been enabled in this repo -- it needs the config files to be merged before GHA can be used in PRs.

@codecov-io
Copy link

codecov-io commented Mar 9, 2020

Codecov Report

Merging #1122 into develop will increase coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1122      +/-   ##
===========================================
+ Coverage    95.77%   95.86%   +0.09%     
===========================================
  Files           49       49              
  Lines         3500     3555      +55     
===========================================
+ Hits          3352     3408      +56     
+ Misses         148      147       -1
Impacted Files Coverage Δ
isort/pylama_isort.py 100% <0%> (ø) ⬆️
isort/sorting.py 100% <0%> (ø) ⬆️
isort/io.py 100% <0%> (ø) ⬆️
isort/output.py 97.97% <0%> (ø) ⬆️
tests/test_io.py 100% <0%> (ø) ⬆️
tests/test_main.py 100% <0%> (ø) ⬆️
tests/test_isort.py 99.67% <0%> (ø) ⬆️
isort/api.py 93.71% <0%> (+0.06%) ⬆️
isort/main.py 84.83% <0%> (+0.21%) ⬆️
isort/settings.py 90% <0%> (+0.43%) ⬆️
... and 2 more

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 44c4100...1a7a967. Read the comment docs.

poetry install

- name: Test
shell: bash

Choose a reason for hiding this comment

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

why specify bash here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

@MCOfficer MCOfficer Mar 13, 2020

Choose a reason for hiding this comment

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

True, but this runs on macOS and ubuntu, which both default to bash. (actually they default to sh, but unless you need something bash-specific that should be fine)

Copy link
Contributor

Choose a reason for hiding this comment

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

Current releases of macOS default to zsh, not bash so I approve of the explicit is better than implicit approach used here.

Choose a reason for hiding this comment

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

it's a nitpick, so i feel fairly indifferent about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Windows would be a great addition for the future as AppVeyor is really slow with a single concurrent job:

Not included:
...

  • Windows. Omitted for now, the poetry install was failing. As this issue is about replacing Travis and not AppVeyor, that can wait.

@timothycrosley timothycrosley merged commit f58f985 into PyCQA:develop May 12, 2020
@hugovk hugovk deleted the add-gha branch May 12, 2020 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants