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

Run unit tests in Github Actions #125

Merged
merged 8 commits into from
Apr 11, 2021
Merged

Run unit tests in Github Actions #125

merged 8 commits into from
Apr 11, 2021

Conversation

krisgesling
Copy link
Contributor

Description:
Adds automated testing for any new PR via Github Actions.

Intentionally commented out flake8 checks at the moment as these will fail on the current codebase.

How to test:
See automated checks below.

CLA

  • Yes

Newer dependencies contain code incompatible with Python 3.5.
@krisgesling krisgesling added enhancement CLA: Yes Contributor License Agreement has been signed labels Apr 5, 2021
Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

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

Looks good and is working, did comment on the flake-8 test which can be partially activated to check for some common mistakes.

.github/workflows/python-app.yml Outdated Show resolved Hide resolved
.github/workflows/python-app.yml Show resolved Hide resolved
.github/workflows/python-app.yml Show resolved Hide resolved
@forslund
Copy link
Collaborator

forslund commented Apr 8, 2021

Discussing abit with @krisgesling yesterday and we've now updated to use test-requirements.txt to install pytest and flake8.

The run_tests.py script is removed for the time being since it's not used anymore. testing instructions have been added to the readme for now. A new script should probably be created to allow running the flake8 and pytest...

README.md Outdated Show resolved Hide resolved
@forslund
Copy link
Collaborator

Since the flake command-line is non-trivial I pulled it out into a run_tests.sh script as you suggested @clusterfudge, readme has been updated with this information.

Copy link
Collaborator

@clusterfudge clusterfudge left a comment

Choose a reason for hiding this comment

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

lgtm!

Thanks @forslund for pulling this across the line. Please open an issue for us to address/document&ignore the lint errors we're excluding with --select=E9,F63,F7,F82

@forslund
Copy link
Collaborator

Will write up an issue during the day.

@forslund forslund merged commit a8be482 into master Apr 11, 2021
@forslund forslund deleted the feature/github-actions branch May 2, 2021 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement has been signed enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants