hi,
bertagaz wrote (02 Sep 2015 10:41:59 GMT) :
> On Tue, Sep 01, 2015 at 06:59:09PM +0200, anonym wrote:
>> On 09/01/2015 12:23 PM, intrigeri wrote:
>> > bertagaz wrote (26 Aug 2015 17:52:26 GMT) :
>> Since pushing stuff into the branch after this field has been set to
>> true invalidates the Jenkins' test suite run, would Jenkins monitor for
>> this and unset the field, or is it up to the committer to unset it?
> Jenkins should probably unset the "Jenkins OK" field by itself if the
> test has failed.
Sure, but this still leaves a window during which a reviewer believes
that the branch has been tested OK, while the test result is about an
older version than the one they're looking at. Racy!
>> I realize this is not a problem unique to this solution. Any way,
>> doing this manually gets hairy since we don't necessarily know which
>> commit Jenkins has tested. I suppose it would help if Jenkins also
>> posted a message about what commit it has successfully tested. Or
>> maybe the field we want to add instead could contain the commit?
> Yes, I think that when Jenkins reports the test result, it should also
> add in a comment some informations. I think at least which commit it
> tested and the link to the test result page.
Yes, and then the reviewer can manually check whether that commit ID
matches what they're looking at. But that doesn't address the race
condition I was mentioning above. It's also not enough info,
see below.
However, if Jenkins *also* unset the "Jenkins OK" field when it
*starts* testing an ISO, then we're fine.
> But as the branch was RfQA, it shouldn't be too complicated to know
> which commit was tested, as unless the test fails, this branch is not
> really supposed to receive new commits.
Well... let's not rely on this. It's quite common that we push a small
fix on top of a RfQA branch, after checking that nobody has started
reviewing it yet.
Also, note that what exactly we're testing is not fully encoded in the
topic branch's HEAD: it also depends on the state of the corresponding
base branch, and of all APT suites / overlays that are listed in
there. So "Jenkins OK for $commit" is a piece of information that is
insufficient, and that needs to expire somehow. Hence the suggestion
of invalidating it at the beginning of each test run.
>> > But this doesn't address the problem anonym pointed to initially, that
>> > is "the reviewer also has to wait until the automated tester posts the
>> > result to the ticket".
>>
>> And the corollary is that it neither solves the problem: reviewers may
>> waste time reviewing a branch that breaks an automated test. That's the
>> important part, IMHO.
Yes. Thankfully this only happens if if the reviewer doesn't wait for
Jenkins to post its results (we're starting looping endlessly on this
one, perhaps :)
>> > One possible solution would be to assign RfQA
>> > tickets to Jenkins initially, and once Jenkins has voted +1 (and set
>> > "Jenkins OK" to true), it could also unassign the ticket from itself,
>> > and then human reviewers can look into it. Jenkins would still run
>> > automated tests on branches regardless of their ticket's assignee, as
>> > specified elsewhere, but at least this would make it clear to human
>> > reviewers when it's time for them to start reviewing stuff.
>>
>> Sure. I guess Jenkins would only assign itself, and not assign the
>> intended human reviewer (since that info isn't available). That seems a
>> bit awkward to me, to (as the implementer) have to return to the tickets
>> when Jenkins is done, and then assign the human reviewer.
Indeed, that's painful. For many branches we don't need to do that as
we don't assign them to a specific reviewer, and the RM is supposed to
handle what's left unreviewed, but still. Another field ("Next
reviewer") could be added and Jenkins would copy its value to the
"Assignee" field, but oh well, ETOOMANYFIELDS, no?
> Maybe when assigning to Jenkins, one can also add herself as a watcher
> to the ticket? Then she would get the ticket update notification in case
> the test passes.
Indeed. One more manual operation, though, unless we find a way to
automatically add to the watchers list anyone who has set QA Check =
Ready for QA.
It feels like we're already feeling the limits of what we're able to
do with Redmine, and trying to use it for stuff that it's not designed
to handle. I'm afraid that any solution we'll come up with will
require some combination of boring, repetitive and error-prone manual
ticket handling, writing custom Redmine plugins, writing custom bots
and/or post-update hooks to update tickets. Maybe it'll soon be time
to look into Zuul, Gerrit and friends for real...
Cheers,
--
intrigeri
