RFR: 1587: Adding review comments should not mean approval status change [v2]

[Erik Joelsson]

On Wed, 19 Oct 2022 19:25:01 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> Zhao Song has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>> 
>>   SKARA-1587
>
> bots/pr/src/test/java/org/openjdk/skara/bots/pr/ReviewersTests.java line 658:
> 
>> 656:             // The pr should still contain 'Ready' label
>> 657:             updatedPr = author.pullRequest(pr.id());
>> 658:             assertTrue(updatedPr.labelNames().contains("ready"));
> 
> I am still wondering why we should update the pr manually.

An instance of `PullRequest` is not guaranteed to reflect the current server state of the pr, it should always be considered a snapshot. This property of a `PullRequest` wasn't properly reflected in the `TestPullRequest` implementation until recently, with the explicit `TestPullRequestStore` object that represents the server side state.

In test code, instead of explicitly fetching an updated snapshot of a PR, you can access the store directly to inspect the state through `pr.store()`. Most old test code has not been updated to use this. Note that the `TestPullRequestStore` doesn't have as many convenience methods for all the data as `TestPullRequest` has, so it may be a bit clunky, at least for labels. You could always add a better accessor (in `TestIssueStore` in this case) to help with that.

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

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