From 5a74e285ac36645d570396d06c28967d955a5c51 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 13 Jan 2016 09:16:14 +0900 Subject: [PATCH] ChangeEditIT: Fix incorrect usage of ExpectedException Having an expected exception at the beginning of a test short circuits the assertions that follow the expect() call. "JUnit uses AssertionErrors for indicating that a test is failing. You have to call assert methods before you set expectations of the ExpectedException rule, if they should be handled by the framework."[1] Remove the assertUnchangedMessage helper method and the two calls to it from the updateMessage test. The first case that was being tested is already covered by the updateMessageNoChange test. Add a new test to cover the other case, where new lines are added onto the edited message. [1]: http://junit.org/apidocs/org/junit/rules/ExpectedException.html Reported-By: Richard Ipsum Change-Id: I565375da7d9511a17bbe1e146c5ab402be1a8045 --- .../gerrit/acceptance/edit/ChangeEditIT.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java index f4e7cb74ea..472f3d28b1 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -317,13 +317,25 @@ public class ChangeEditIT extends AbstractDaemonTest { edit.get().getEditCommit().getFullMessage()); } + @Test + public void updateMessageOnlyAddTrailingNewLines() throws Exception { + assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId))) + .isEqualTo(RefUpdate.Result.NEW); + Optional edit = editUtil.byChange(change); + + exception.expect(UnchangedCommitMessageException.class); + exception.expectMessage( + "New commit message cannot be same as existing commit message"); + modifier.modifyMessage( + edit.get(), + edit.get().getEditCommit().getFullMessage() + "\n\n"); + } + @Test public void updateMessage() throws Exception { assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId))) .isEqualTo(RefUpdate.Result.NEW); Optional edit = editUtil.byChange(change); - assertUnchangedMessage(edit, edit.get().getEditCommit().getFullMessage()); - assertUnchangedMessage(edit, edit.get().getEditCommit().getFullMessage() + "\n\n"); String msg = String.format("New commit message\n\nChange-Id: %s\n", change.getKey()); assertThat(modifier.modifyMessage(edit.get(), msg)).isEqualTo( @@ -672,14 +684,6 @@ public class ChangeEditIT extends AbstractDaemonTest { assertThat(approvals.get(0).value).isEqualTo(1); } - private void assertUnchangedMessage(Optional edit, String message) - throws Exception { - exception.expect(UnchangedCommitMessageException.class); - exception.expectMessage( - "New commit message cannot be same as existing commit message"); - modifier.modifyMessage(edit.get(), message); - } - @Test public void testHasEditPredicate() throws Exception { assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);