RFR: 1431: The method TestPullRequest#diff sometimes returns wrong information

[Erik Joelsson]

On Mon, 22 Aug 2022 07:51:04 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> Hi all,
>> 
>> The method `TestPullRequest#diff` always passes the head hash of the target branch to the method `Repository#diff` as the base hash. But it may be wrong when the target branch was submitted some new commits after creating the pull request. `TestPullRequest#diff` should find the last common hash of the source and target branch as the base hash.
>> 
>> This patch fixes the bug and adds the test case. I don't know whether it is good to place the test case at the class `CSRBotTests`, but the method `CSRBotTests#testBackportCsr` indeed contains the situation of this bug.
>> 
>> Thanks for taking the time to review.
>> 
>> Best Regards,
>> -- Guoxiong
>
> test/src/main/java/org/openjdk/skara/test/TestPullRequest.java line 257:
> 
>> 255:             var baseHash = sourceHash;
>> 256:             var targetBranch = new Branch(targetRef());
>> 257:             while (!"0".repeat(40).equals(baseHash.hex())) {
> 
> So if the common commit is more than 40 commits away, return "no diff"? :-)
> 
> This is only used in testing, right? If so, this you could get away with it, but I think a comment describing the limitations would be needed.

I think you are misunderstanding the code. `"0".repeat(40)` just generates a string with 40 zeros, basically a null hash.

Looping to find the common ancestor is one way of doing it, but I would recommend checking out `git merge-base`. There is a method for it already on `GitRepository`.

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

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