opendev/gerrit
Code Issues Proposed changes

ChangeEditIT: Fix incorrect usage of ExpectedException

Browse Source 
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 <richard.ipsum@codethink.co.uk>
Change-Id: I565375da7d9511a17bbe1e146c5ab402be1a8045
This commit is contained in:
David Pursehouse
2016-01-13 09:16:14 +09:00
parent 92112508b3
commit 5a74e285ac
1 changed files with 14 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
View File

@@ -317,13 +317,25 @@ public class ChangeEditIT extends AbstractDaemonTest {
edit.get().getEditCommit().getFullMessage()); edit.get().getEditCommit().getFullMessage());
} }
@Test
public void updateMessageOnlyAddTrailingNewLines() throws Exception {
assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))
.isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> 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 @Test
public void updateMessage() throws Exception { public void updateMessage() throws Exception {
assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId))) assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))
.isEqualTo(RefUpdate.Result.NEW); .isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> edit = editUtil.byChange(change); Optional<ChangeEdit> 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", String msg = String.format("New commit message\n\nChange-Id: %s\n",
change.getKey()); change.getKey());
assertThat(modifier.modifyMessage(edit.get(), msg)).isEqualTo( assertThat(modifier.modifyMessage(edit.get(), msg)).isEqualTo(
@@ -672,14 +684,6 @@ public class ChangeEditIT extends AbstractDaemonTest {
assertThat(approvals.get(0).value).isEqualTo(1); assertThat(approvals.get(0).value).isEqualTo(1);
} }
private void assertUnchangedMessage(Optional<ChangeEdit> 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 @Test
public void testHasEditPredicate() throws Exception { public void testHasEditPredicate() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW); assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);