RFR: 1589: Improve TestIssue and TestPullRequest [v2]

[Erik Joelsson]

On Fri, 9 Sep 2022 15:03:09 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:
>> 
>>   Unused imports
>
> issuetracker/src/test/java/org/openjdk/skara/issuetracker/IssueTrackerTests.java line 130:
> 
>> 128:             var links = issue2.links();
>> 129:             assertEquals(1, links.size());
>> 130:             assertEquals(link.relationship(), links.get(0).relationship());
> 
> Here's an obvious drawback of the "var" syntax. What type is this `links`? Why did not an ordinary equals suffice? Does this really relate to the rest of the changes, or is it an independent fix that happened to tag along?

Yes, in modern intellij, I get grey type names for every `var` variable, which makes it really convenient. I too have to open most PRs I review in the IDE to properly read the code.

Regarding why equals won't suffice. Before this patch, `TestIssue::links` would just return a list of all `Link` objects that had been added to the Issue. In this patch, I had to change it to always refresh the target `Issue` of each link with a fresh snapshot, because that's what happens when you query for any real links. And since Link is read-only that means creating a new `Link` instance each time as well. Another way to solve this would be to implement `Link::equals` to take this into account. That would probably be a good thing to do, but then this patch will touch product code. Long term, that's probably better and I can do that if you want.

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

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