-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add software tests and CI configuration #39
base: master
Are you sure you want to change the base?
Conversation
|
||
lint: setup-virtualenv | ||
$(pip) install --requirement=requirements-utils.txt | ||
$(flake8) --exit-zero *.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With --exit-zero
, it will currently not bail out on any findings. We will satisfy the linters with a subsequent patch.
$(MAKE) proselint | ||
|
||
proselint: | ||
$(proselint) *.md || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Let's start in relaxed mode. || true
will turn any failure exit code into a positive outcome.
Hi Frederic, I think this will be good to go. While the few test cases only bring in a code coverage of about 10%, at least the infrastructure will be in place. I will submit another patch which exercises the code base more thoroughly 1. With kind regards, Footnotes
|
Hi Frederic, it may take a few more days to get my machine fixed, so I think all other work on this topic can be submitted with subsequent patches. I think it is good for a patch, mainly focused on enabling CI, anyway. In other words: Feel free to merge, in order to get the test infrastructure in place. With kind regards, |
Hi Frederic, my machine got fixed quickly, so I will be able to continue to work a bit on the test harness. Do you have any objections against merging this PR as a baseline beforehand? I will submit subsequent work as separate PRs. With kind regards, |
The followup patch will be cicerops/check_synology@add-ci...more-tests, submitted with #40 as a draft, bumping the code coverage from 10% to 27% by exercising the |
Thx for that contribution. Sadly i'm super short on time in the last weeks - I promise i did not forget this addition but i would like to go through everything in detail before merging - and this needs time |
Dear Frederic, thank you. I remembered this patch yesterday as well, sweet that we apparently both did at about the same time ;]. Please take your time to review the patch properly. With kind regards, |
Just run `git clone ...` and `make test`.
ERROR: For req: check-synology==1.0.0. Invalid script entry point: <ExportEntry check_synology = check_synology:None []> - A callable suffix is required. Cf https://packaging.python.org/specifications/entry-points/#use-for-scripts for more information.
Dear Frederic,
this patch will add some software tests and a CI/GHA configuration. It works well already, see below. I believe it will tremendously help with future maintenance.
With kind regards,
Andreas.
-- https://github.com/cicerops/check_synology/actions/runs/2777733734