Skip to content

Commit a222a78

Browse files
committed
Fix rejection of staged changes after successful integration
Staging ref rebuild logic did not take into account that current branch head is now always ancestor of the old staging ref Fixes: QTQAINFRA-4384 Change-Id: I13d47219119abd7add54fd85354c02503718bc55 Reviewed-by: Paul Wicking <[email protected]> Reviewed-by: Daniel Smith <[email protected]>
1 parent 17121cc commit a222a78

File tree

2 files changed

+29
-2
lines changed

2 files changed

+29
-2
lines changed

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ private List<ChangeData> arrangeOrderLikeInRef(
313313
if (refObj.equals(tipObj)) return results;
314314

315315
RevWalk revWalk = new RevWalk(git);
316+
RevCommit branchHead = revWalk.parseCommit(tipObj);
316317
RevCommit commit = revWalk.parseCommit(refObj);
317318
int count = 0;
318319
do {
@@ -331,7 +332,7 @@ private List<ChangeData> arrangeOrderLikeInRef(
331332
// It can always be trusted that parent in index 0 is the correct one
332333
commit = revWalk.parseCommit(commit.getParent(0));
333334
}
334-
} while (commit != null && !commit.equals(tipObj) && count < 100);
335+
} while (commit != null && !revWalk.isMergedInto(commit, branchHead) && count < 100);
335336

336337
if (count == 100) return null;
337338
return results;
@@ -376,7 +377,10 @@ private ObjectId findReusableStagingHead(
376377

377378
ObjectId reusableHead = null;
378379
RevWalk revWalk = new RevWalk(git);
380+
RevCommit branch = revWalk.parseCommit(branchHead);
379381
RevCommit commit = revWalk.parseCommit(stagingHead);
382+
if (!revWalk.isMergedInto(branch, commit)) return branchHead;
383+
380384
int count = 0;
381385
do {
382386
count++;

src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import static com.google.common.truth.Truth.assertThat;
66
import static com.google.common.truth.Truth.assertWithMessage;
7+
import static com.google.gerrit.acceptance.GitUtil.assertPushOk;
8+
import static com.google.gerrit.acceptance.GitUtil.pushHead;
79
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
810
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
911
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
@@ -54,6 +56,8 @@ public class QtCodeReviewIT extends LightweightPluginDaemonTest {
5456
protected static final String BUILD_PASS_MESSAGE = "thebuildpassed";
5557
protected static final String BUILD_FAIL_MESSAGE = "thebuildfailed";
5658

59+
private RevCommit lastGeneratedCommit;
60+
5761
@Before
5862
public void ReduceLogging() throws Exception {
5963
LogManager.resetConfiguration();
@@ -72,6 +76,25 @@ public void ReduceLogging() throws Exception {
7276
root.addAppender(dst);
7377
}
7478

79+
@Before
80+
public void GenerateCommits() throws Exception {
81+
// Generate 100+ commits to match more like production environment
82+
RevCommit parent = getRemoteHead();
83+
for (int i = 0; i <= 101; i++) {
84+
RevCommit c = testRepo.commit()
85+
.message("a commit " + String.valueOf(i))
86+
.add("afile" + String.valueOf(i), "somecontent")
87+
.parent(parent)
88+
.insertChangeId()
89+
.create();
90+
testRepo.reset(c);
91+
assertPushOk(pushHead(testRepo, "refs/heads/master", false), "refs/heads/master");
92+
parent = c;
93+
}
94+
lastGeneratedCommit = parent;
95+
resetEvents();
96+
}
97+
7598
@Test
7699
public void dummyTest() throws Exception {}
77100

@@ -296,7 +319,7 @@ protected void assertApproval(String changeId, TestAccount user) throws Exceptio
296319
protected void assertReviewedByFooter(RevCommit commit, boolean exists) {
297320

298321
// Skip initial commit and merge commits
299-
if (commit.getParentCount() != 1) return;
322+
if (commit.getParentCount() != 1 || commit.equals(lastGeneratedCommit)) return;
300323

301324
List<String> changeIds = commit.getFooterLines(FooterConstants.REVIEWED_BY);
302325
assertThat(!changeIds.isEmpty()).isEqualTo(exists);

0 commit comments

Comments
 (0)