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) :
> >> On Wed, Aug 26, 2015 at 03:38:19PM +0200, anonym wrote:
> >>> The current proposal seems to be to only start the automated test run of
> >>> a feature branch when it is marked "Ready for QA". This has overloaded
> >>> the meaning of this status so it no longer alone means that the branch
> >>> is ready for review'n'merge; the reviewer also has to wait until the
> >>> automated tester posts the result to the ticket.
> >>>
> >>> We could get rid of this ambiguity by splitting "Ready for QA" into
> >>> "Ready for (automated) testing" (RFT) and "Ready for review" (RFR). Example:
> >>>
> >>> Let's say I have created a new feature branch and think I'm done. I
> >>> assign it to intrigeri (who I want to review it in the end) and mark it
> >>> RFT. And automated build is triggered, and then a full run of the test
> >>> suite. Let's say the test suite fails. Then it gets reassigned to me and
> >>> marked "Dev needed". I fix the issue (which probably was easier to spot
> >>> for me than it would be for intrigeri, since I did the implementation)
> >>> assign it to intrigeri and mark it RFT again. This time the test suite
> >>> passes, and then the ticket is marked RFR automatically. Also, if I run
> >>> the test suite myself and see it pass, then I can just mark it RFR directly.
> >
> >> I've think about this issue too. My own conclusion was to add a new
> >> Custom fiels in redmine, that Jenkins would use, so something similar to
> >> yours. The dev mark the ticket as ReadyForQA, then Jenkins run the test
> >> suite on it and send its report modifying that field accordingly.
> >
> > The way it's conceptualized in Gerrit (and presumably in other similar
> > tools) is that Jenkins is "just another reviewer", that can vote +1
> > or -1 on a merge request. I like this logic. To translate it into
> > Redmine, RfQA means that all reviewers (humans and robots) can start
> > looking at the branch, and Pass means that all reviewers are happy
> > with it.
>
> Conceptually, I like this.
>
> > I think it would be overly complicated to encode individual
> > reviews (e.g. the one done by Jenkins) in the QA Check field, and
> > conceptually I prefer to keep QA Check a bit more high level and not
> > give it a finer granularity.
>
> I certainly agree that my proposal complicates things.
>
> > So, adding dedicated custom fields seems to be the best option to
> > encode individual review results: "Jenkins OK" would be unset
> > initially, and set to true (= +1) upon successful testing. Upon failed
> > testing:
> >
> > * "QA Check" would be set back to "Dev Needed";
> > * a negative vote from Jenkins is a blocker, so given QA Check has
> > been reset already, I'm not sure it's useful to also set "Jenkins
> > OK" to "false" (which we would have to revert after pushing a fix
> > and setting QA Check = RfQA).
> >
> > ... and "Human reviewer OK" would work just the same.
>
> 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.
> 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.
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.
> > 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.
>
> > 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.
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.
bert.
