On dt., set. 24 2019, jaromil@??? wrote:

> On Sat, 21 Sep 2019, Mark Hindley wrote:
>
>> Have you a way of testing this before it is put into
>> production?
>
> this is a very good question. I've been asking for testing
> coverage
> for amprolla since ages. Feel free to call me conservative, but
> I do
> believe the only code that can last the challenge of time is
> code that
> has extensive test coverage, both at the level of units and
> integration.
>
> IMHO test coverage should be the priority for amprolla's
> development
> and it shouldn't be so hard to realise given the relative
> simplicity
> of the software and the easily scriptable entry point.


A couple things:

- Asking everyone to review this change and allowing ample time to
do so, is a huge improvement against the way others have
modified critical bits of the infra in the past. Including
Amprolla.
- I did test that the code runs and behaves as expected (or I
wouldn't have proposed it), but "in production" in this case
basically would imply that I run a Devuan-derivative with users,
which I don't have the time for or the will to do so.
- While tests are good, they are not a pursuit-worthy goal *in and
on itself*. Specially when something is so obviously implemented
in a non-satisfactory fashion.
- This reads as: "Unless there are tests, *nobody* should be
changing *anything* on critical infra". Which isn't all that
bad. If it is for *everyone* and *everything*.
- OTOH such a strong policy can be detrimental to small
incremental improvements as the overhead of adding tests is
non-negligible if the code was not written with tests in mind.
E.g. what rrq mentioned about the code not being shareable
between the multiple Amprolla instances because of the
configuration makes testing more complicated.
The fact that the orchestration script is de-facto part of the
working of Amprolla and has Devuan-infra things hard-coded,
makes testing more complicated as well.
A patch that would start supporting testing would likely require
non negligible modification of existing code and raises the bar
quite high to all the low-hanging fruits in Amprolla atm (like
locking, or a potential endless loop, or, or, ...); while small
reviewable (and reviewed) improvements to the code-base that
make it easier to add tests afterwards are certainly easier.
- Such a policy has never been discussed AFAIK and, TBH (with the
rest of *this point* being less factual and more human), raising
it to that extent, when the process is a clear improvement over
the past, feels like blocking for blocking's sake (or because of
who proposed the patch).
- That being said, I have been wanting to add monitoring for when
Amprolla locks and stops updating; I consider it more important
to check externally that the system (Amprolla in this case) is
working fine, than it is to have tests that only check for what
whoever wrote the tests thought of checking. E.g. code that is
"correct" and passes tests with 100% code coverage but has a
lousy locking logic, can still be broken in production from time
to time.
- Even if this patch never lands, because atm I am not willing to
invest the time to add extensive testing for someone else's
code, it doesn't look like others are willing to touch Amprolla,
and there is no way I'm touching it without clear consensus;
that monitoring should be a good canary for "Amprolla is stuck
again" and still improve things so that someone gives it a kick
without having someone log in periodically and manually check
(as is the case now).
- Since I run all monitoring on external infra of my own, I can
just go ahead and do that when I have the time, no need for
consensus there :-).
--
Evilham