Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test github actions for CI #46

Closed
tdowrick opened this issue Apr 30, 2020 · 14 comments · Fixed by #48
Closed

test github actions for CI #46

tdowrick opened this issue Apr 30, 2020 · 14 comments · Fixed by #48
Assignees

Comments

@tdowrick
Copy link
Contributor

Windows/Mac/Linux

@tdowrick tdowrick self-assigned this Apr 30, 2020
@tdowrick
Copy link
Contributor Author

@MattClarkson @thompson318 @mianasbat

See https://github.com/UCL/scikit-surgerycore/blob/github-actions-tox/.github/workflows/pythonapp.yml for example config file to run tests on mac/windows/linux. If you go to the 'Actions' tab in the repo you can see the results of the run(s).

@thompson318
Copy link
Contributor

Looks about right, thank you. It's good that it's using tox. I couldn't find the badges, but I assume they're somewhere. So if we can set them up, and deploy to PyPi, then remove travis.yml and gitlab.yml we've got a nicely migrated repository.

tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
tdowrick added a commit that referenced this issue May 1, 2020
…er naming for untagged commits, not worth fixing (for now)
@tdowrick
Copy link
Contributor Author

tdowrick commented May 1, 2020

  • I've added in PyPI deployment, and tested with release 0.6.2.
  • The badge should be set up to work, but won't be visible until we merge to master, as it looks for the CI results from the master branch by default.
  • travis and gitlab CI files removed.

I have used an API token, rather than username/password for authentication with PyPI, which is now the recommended method of authenticating it seems. The token is stored as a GitHub secret on the repo. It doesn't look like it is possible to share token between repos, so we will have to add it manually to other repos as we go along.

As a side note, I read that GitHub actions are only free for public repos - are we likely to put any private ones on GitHub, or will we be keeping those on Gitlab?

@thompson318
Copy link
Contributor

My approach would be to only put public repos on github. If it's something you want to keep private then you might as well leave it on gitlab.

@thompson318
Copy link
Contributor

Is this ready to be merged?

@tdowrick
Copy link
Contributor Author

tdowrick commented May 5, 2020

Yes - then we can check that the status badge is working properly, and if so, close the issue.

@tdowrick tdowrick closed this as completed May 5, 2020
@thompson318 thompson318 reopened this May 5, 2020
@thompson318
Copy link
Contributor

I just reopened this for the coverage button. The coverage badge links to coveralls.io which was done with

after_success:

  • coveralls

in travis. The coverage badge still links to coveralls, but I don't think the workflow is setting it now? Is there a better way to report coverage with the github workflow?

@tdowrick
Copy link
Contributor Author

tdowrick commented May 5, 2020

I'll take a look

@tdowrick
Copy link
Contributor Author

tdowrick commented May 5, 2020

Doesn't seem like there is any straightforward way to do it. We'd have to generate the coverage report within tox, which requires a bit of fiddling to get working. I shall keep at it.

@thompson318
Copy link
Contributor

What happens if you just run "coveralls" after tox? All the travis.yml was doing was

coverage run -a --source ./sksurgerycore -m pytest -v -s ./tests/

coverage report -m

then coveralls

@tdowrick
Copy link
Contributor Author

tdowrick commented May 5, 2020

I think if you run it after tox it won't have access to the coverage report (which is run in tox), but I can check.

@tdowrick
Copy link
Contributor Author

tdowrick commented May 5, 2020

@thompson318
Copy link
Contributor

Excellent, thank you. I did put coveralls in requirements-dev, so pip install coveralls may not be necessary, alternatively remove coveralls from requirements-dev.

@tdowrick
Copy link
Contributor Author

tdowrick commented May 5, 2020

Requirements-dev is only installed in the tox environment, so we need the coveralls import as well, but we can take it out of requirements-dev.

The alternative is to install requirements-dev in the script, but that seemed unnecessary just to get tox and coveralls.

@tdowrick tdowrick mentioned this issue May 7, 2020
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 a pull request may close this issue.

2 participants