search

wernerfred / check_synology

This plugin will check a lot of different values on your Synology DiskStation.
MIT License
17 stars 25 forks source link

Add software tests and CI configuration #39

Open amotl opened 2 years ago

amotl commented 2 years ago

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.

image -- https://github.com/cicerops/check_synology/actions/runs/2777733734

amotl commented 2 years ago

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, Andreas.

[^1]: May take some time, have to get my machine fixed beforehand and vacation is near.

amotl commented 2 years ago

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, Andreas.

amotl commented 2 years ago

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, Andreas.

amotl commented 2 years ago

The followup patch will be https://github.com/cicerops/check_synology/compare/add-ci...more-tests, submitted with #40 as a draft, bumping the code coverage from 10% to 27% by exercising the load mode for real. It will have to be extended to exercise all other modes and cleaned up a bit before submitting it.

wernerfred commented 1 year ago

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

amotl commented 1 year ago

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, Andreas.

amotl commented 1 year ago

Friendly bump ;]. As a few reports about potential edge cases are arriving on the issue tracker, and because there are a few contributions not merged yet, like #9, #41 and #46 (thanks a stack, @dommi22m and @bigitag!), I think it would be good to have a test harness in place.