Merge branch 'stable-2.16' into stable-3.0 * stable-2.16: Do not reject commit upon connectivity problems Change-Id: Ie4a844b61782f0b8118d0014696ad933a60820a2
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java index f02050b..b0c9e39 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
@@ -47,6 +47,12 @@ @Inject private IssueExtractor issueExtractor; + private enum ItsExistenceCheckResult { + EXISTS, + DOESNT_EXIST, + CONNECTIVITY_FAILURE + } + private List<CommitValidationMessage> validCommit(Project.NameKey project, RevCommit commit) throws CommitValidationException { List<CommitValidationMessage> ret = Lists.newArrayList(); @@ -63,16 +69,23 @@ List<String> nonExistingIssueIds = Lists.newArrayList(); client = itsFacadeFactory.getFacade(project); for (String issueId : issueIds) { - boolean exists = false; + ItsExistenceCheckResult existenceCheckResult; try { - exists = client.exists(issueId); + existenceCheckResult = + client.exists(issueId) + ? ItsExistenceCheckResult.EXISTS + : ItsExistenceCheckResult.DOESNT_EXIST; } catch (IOException e) { - synopsis = "Failed to check whether or not issue " + issueId + " exists"; + synopsis = + "Failed to check whether or not issue " + + issueId + + " exists, due to connectivity issue. Commit will be accepted."; log.warn(synopsis, e); details = e.toString(); - ret.add(commitValidationFailure(synopsis, details)); + existenceCheckResult = ItsExistenceCheckResult.CONNECTIVITY_FAILURE; + ret.add(commitValidationFailure(synopsis, details, existenceCheckResult)); } - if (!exists) { + if (existenceCheckResult == ItsExistenceCheckResult.DOESNT_EXIST) { nonExistingIssueIds.add(issueId); } } @@ -95,7 +108,7 @@ sb.append(" Issue-Tracker"); details = sb.toString(); - ret.add(commitValidationFailure(synopsis, details)); + ret.add(commitValidationFailure(synopsis, details, ItsExistenceCheckResult.DOESNT_EXIST)); } } else if (!itsConfig .getDummyIssuePattern() @@ -118,7 +131,7 @@ sb.append(" Issue-Tracker"); details = sb.toString(); - ret.add(commitValidationFailure(synopsis, details)); + ret.add(commitValidationFailure(synopsis, details, ItsExistenceCheckResult.DOESNT_EXIST)); } break; case OPTIONAL: @@ -128,10 +141,12 @@ return ret; } - private CommitValidationMessage commitValidationFailure(String synopsis, String details) + private CommitValidationMessage commitValidationFailure( + String synopsis, String details, ItsExistenceCheckResult existenceCheck) throws CommitValidationException { CommitValidationMessage ret = new CommitValidationMessage(synopsis + "\n" + details, false); - if (itsConfig.getItsAssociationPolicy() == ItsAssociationPolicy.MANDATORY) { + if (itsConfig.getItsAssociationPolicy() == ItsAssociationPolicy.MANDATORY + && existenceCheck != ItsExistenceCheckResult.CONNECTIVITY_FAILURE) { throw new CommitValidationException(synopsis, Collections.singletonList(ret)); } return ret;
diff --git a/src/main/resources/Documentation/config-common.md b/src/main/resources/Documentation/config-common.md index 666481f..109aba4 100644 --- a/src/main/resources/Documentation/config-common.md +++ b/src/main/resources/Documentation/config-common.md
@@ -43,7 +43,9 @@ MANDATORY : One or more issue-ids are required in the git commit message. The git push will - be rejected otherwise. + be rejected otherwise. Note that in case of connectivity issues with ITS, + the commit will be accepted. The client will be notified about the + connectivity issue and the result. SUGGESTED : Whenever the git commit message does not contain one or more issue-ids,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java index d247a7c..666d463 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateCommentTest.java
@@ -468,20 +468,21 @@ assertEquals("Size of returned CommitValidationMessages does not match", 2, ret.size()); assertTrue( - "First CommitValidationMessages does not contain " + "'Failed'", - ret.get(0).getMessage().contains("Failed")); + "First CommitValidationMessages does not contain " + "'Failed to check'", + ret.get(0).getMessage().contains("Failed to check")); assertTrue( "First CommitValidationMessages does not contain '4711'", ret.get(0).getMessage().contains("4711")); + assertTrue( + "First CommitValidationMessages contains reason of failure", + ret.get(0).getMessage().contains("InjectedEx1")); assertFalse( "First CommitValidationMessages contains '42', although " + "that bug exists", ret.get(0).getMessage().contains("42")); assertTrue( "Second CommitValidationMessages does not contain " + "'Non-existing'", ret.get(1).getMessage().contains("Non-existing")); - assertTrue( - "Second CommitValidationMessages does not contain '4711'", - ret.get(1).getMessage().contains("4711")); + assertTrue( "Second CommitValidationMessages does not contain '42'", ret.get(1).getMessage().contains("42")); @@ -489,6 +490,114 @@ assertLogMessageContains("4711"); } + public void testMandatoryMatchingSingleIOException() + throws CommitValidationException, IOException { + List<CommitValidationMessage> ret; + ItsValidateComment ivc = injector.getInstance(ItsValidateComment.class); + ReceiveCommand command = createMock(ReceiveCommand.class); + RevCommit commit = createMock(RevCommit.class); + CommitReceivedEvent event = newCommitReceivedEvent(command, project, null, commit, null); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY) + .atLeastOnce(); + expect(commit.getFullMessage()).andReturn("bug#4711").atLeastOnce(); + expect(commit.getId()).andReturn(commit).anyTimes(); + expect(commit.getName()).andReturn("TestCommit").anyTimes(); + expect(issueExtractor.getIssueIds("bug#4711")).andReturn(new String[] {"4711"}).atLeastOnce(); + expect(itsFacadeFactory.getFacade(project.getNameKey())).andReturn(itsFacade).anyTimes(); + expect(itsFacade.exists("4711")).andThrow(new IOException("InjectedEx1")).atLeastOnce(); + + replayMocks(); + + ret = ivc.onCommitReceived(event); + + assertEquals("Size of returned CommitValidationMessages does not match", 1, ret.size()); + assertTrue( + "First CommitValidationMessages does not contain " + "'Failed to check'", + ret.get(0).getMessage().contains("Failed to check")); + assertTrue( + "First CommitValidationMessages does not contain '4711'", + ret.get(0).getMessage().contains("4711")); + assertTrue( + "First CommitValidationMessages contains reason of failure", + ret.get(0).getMessage().contains("InjectedEx1")); + + assertLogMessageContains("4711"); + } + + public void testMandatoryMatchingMultipleIOExceptionIsNonExisting() throws IOException { + List<CommitValidationMessage> ret; + ItsValidateComment ivc = injector.getInstance(ItsValidateComment.class); + ReceiveCommand command = createMock(ReceiveCommand.class); + RevCommit commit = createMock(RevCommit.class); + CommitReceivedEvent event = newCommitReceivedEvent(command, project, null, commit, null); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY) + .atLeastOnce(); + expect(commit.getFullMessage()).andReturn("bug#4711, bug#42").atLeastOnce(); + expect(commit.getId()).andReturn(commit).anyTimes(); + expect(commit.getName()).andReturn("TestCommit").anyTimes(); + expect(issueExtractor.getIssueIds("bug#4711, bug#42")) + .andReturn(new String[] {"4711", "42"}) + .atLeastOnce(); + expect(itsFacadeFactory.getFacade(project.getNameKey())).andReturn(itsFacade).anyTimes(); + expect(itsFacade.exists("4711")).andThrow(new IOException("InjectedEx1")).atLeastOnce(); + expect(itsFacade.exists("42")).andReturn(false).atLeastOnce(); + + replayMocks(); + + try { + ivc.onCommitReceived(event); + fail("onCommitReceived did not throw any exception"); + } catch (CommitValidationException e) { + assertLogMessageContains("Failed to check whether or not issue 4711 exists"); + assertTrue( + "Message of thrown CommitValidationException does not " + "contain 'Non-existing'", + e.getMessage().contains("Non-existing")); + } + } + + public void testMandatoryMatchingMultipleIOExceptionExisting() + throws CommitValidationException, IOException { + List<CommitValidationMessage> ret; + ItsValidateComment ivc = injector.getInstance(ItsValidateComment.class); + ReceiveCommand command = createMock(ReceiveCommand.class); + RevCommit commit = createMock(RevCommit.class); + CommitReceivedEvent event = newCommitReceivedEvent(command, project, null, commit, null); + + expect(itsConfig.getItsAssociationPolicy()) + .andReturn(ItsAssociationPolicy.MANDATORY) + .atLeastOnce(); + expect(commit.getFullMessage()).andReturn("bug#4711, bug#42").atLeastOnce(); + expect(commit.getId()).andReturn(commit).anyTimes(); + expect(commit.getName()).andReturn("TestCommit").anyTimes(); + expect(issueExtractor.getIssueIds("bug#4711, bug#42")) + .andReturn(new String[] {"4711", "42"}) + .atLeastOnce(); + expect(itsFacadeFactory.getFacade(project.getNameKey())).andReturn(itsFacade).anyTimes(); + expect(itsFacade.exists("4711")).andThrow(new IOException("InjectedEx1")).atLeastOnce(); + expect(itsFacade.exists("42")).andReturn(true).atLeastOnce(); + + replayMocks(); + + ret = ivc.onCommitReceived(event); + + assertEquals("Size of returned CommitValidationMessages does not match", 1, ret.size()); + assertTrue( + "First CommitValidationMessages does not contain " + "'Failed to check'", + ret.get(0).getMessage().contains("Failed to check")); + assertTrue( + "First CommitValidationMessages does not contain '4711'", + ret.get(0).getMessage().contains("4711")); + assertTrue( + "First CommitValidationMessages contains reason of failure", + ret.get(0).getMessage().contains("InjectedEx1")); + + assertLogMessageContains("4711"); + } + public void assertEmptyList(List<CommitValidationMessage> list) { if (!list.isEmpty()) { StringBuffer sb = new StringBuffer();