RFR: DRAFT: 1565: Only poll for updated MRs from GitLab
erikj at openjdk.org
Fri Sep 2 22:32:52 UTC 2022
On Fri, 2 Sep 2022 22:08:08 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
> This is another rather big patch, but again, with some explanation, it shouldn't be too scary. The most important part is the new `PullRequestPoller` class. This class implements a polling mechanism for PullRequests that tries to only return new or updated pull requests to a bot and guaranteeing that none are missed. The goal is to minimize unnecessary evaluations that currently take place, especially when polling Gitlab. I have added quite a few tests to verify (almost) all the functionality of the poller. For a more detailed explanation of all the challenges this poller faces, see the bug description.
> In order to reduce impact, I'm only making one existing bot use the new poller, the CSRPullRequestBot. I picked this one as it should have pretty low impact overall if something goes wrong. I will followup with further conversions of other bots. I'm also touching the `CSRIssueBot` to introduce the same kind of error handling with retry that the new `PullRequestPoller` implements. I hadn't realized before that this error handling was missing in the CSRBots, and to be able to implement it properly for the `CSRPullRequestBot`, I had to also add the support for the issue polling `CSRIssueBot`.
> There are features of the poller that aren't in use yet. I tried to look through all future users to make sure I could cover all existing usecases from the start.
> Most of the rest of the changes are just adding `equals` and `hashCode` to a bunch of classes that we now need to compare (to know if a PR has been updated or not). This includes the `GitHubPullrequest`, `GitLabMergeRequest` and all the JSON types. For test code this also includes the `TestPullRequest` and `TestIssue` and their related data classes.
> In order to test this properly, I needed to implement a way to for the `TestHostedRepository` to return copies of `TestPullRequest` instances from queries. This better simulates how new instances are always created when `PullRequest`s are queried from a remote `Forge`. The existing bot tests don't need it, but without this, it's not possible to verify the behavior of the poller. I tried a lot of different ways of implementing clone/copy (Object.clone(), copy constructor and specialized copy methods), but what I have here is what I think worked best in the end, with the least impact on existing code.
Please disregard this for now. I discovered a testing issue with any bot using the poller that I need to address properly first.
More information about the skara-dev