RFR: 1659: Fix polling and retries in testinfo bot [v2]

[Magnus Ihse Bursie]

On Tue, 8 Nov 2022 10:22:58 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments
>
> bots/testinfo/src/main/java/org/openjdk/skara/bots/testinfo/TestInfoBot.java line 63:
> 
>> 61:                 .filter(pr -> pr.sourceRepository().isPresent())
>> 62:                 .map(pr -> (WorkItem) new TestInfoBotWorkItem(pr,
>> 63:                         delay -> poller.retryPullRequest(pr, now.plus(delay))))
> 
> Maybe I'm getting lost here, but this feels wrong to me. The `now` is computed as the now of the call to `getPeriodicItems()`, but it is used in the lambda for the `retry` operation of `TestInfoBotWorkItem`. But that retry should happen `delay` time after it is evaluated, right? 
> 
> It might not have much practical impact, but it just looks odd to me, like mixing different time streams.

To expand a bit: the `now.plus(delay)` will be evaluated in the `run` method of the work item, but then the result will be offset not from the time of `run` but from the time of the call to this method. What would happen if it, for some reason, would take more than 2 minutes between these two events, and we'd get a time in the past. Will it be retried immediately, or will it cause trouble with the poller? 

And also it feel conceptually wrong; the retry time feels like it should be specified from when the decision is made for the next check.

-------------

PR: https://git.openjdk.org/skara/pull/1411