Permalink
Browse files

[Fix JENKINS-16737] and [Fix JENKINS-10434] - no exception if author …

…not found

Return unknown user rather than throwing exception in GitChangeSet

Refer to [JENKINS-16737] and [JENKINS-10434] for two cases where a
GitChangeSet cannot find an author and throws a RuntimeException when
it would be much better to report an unknown user and allow execution
to continue.

Added a Polish character to the accented character test

Use more diacritics in the latin accented character test

Use parameterized GitChangeSet test to better cover cases
  • Loading branch information...
MarkEWaite committed Oct 11, 2014
1 parent 6610db3 commit 287e5434d790c9a1c7204e38a09cb843b209a3d7
@@ -247,6 +247,9 @@ public String getParentCommit() {
*/
public User findOrCreateUser(String csAuthor, String csAuthorEmail, boolean createAccountBasedOnEmail) {
User user;
if (csAuthor == null) {
return User.getUnknown();
}
if (createAccountBasedOnEmail) {
user = User.get(csAuthorEmail, false);

@@ -297,8 +300,12 @@ private boolean hasHudsonTasksMailer() {
}
}

private boolean isCreateAccountBasedOnEmail() {
DescriptorImpl descriptor = (DescriptorImpl) Hudson.getInstance().getDescriptor(GitSCM.class);
private boolean isCreateAccountBasedOnEmail() {
Hudson hudson = Hudson.getInstance();
if (hudson == null) {
return false;
}
DescriptorImpl descriptor = (DescriptorImpl) hudson.getDescriptor(GitSCM.class);

return descriptor.isCreateAccountBasedOnEmail();
}
@@ -319,10 +326,6 @@ public User getAuthor() {
csAuthorEmail = this.committerEmail;
}

if (csAuthor == null) {
throw new RuntimeException("No author in changeset " + id);
}

return findOrCreateUser(csAuthor, csAuthorEmail, isCreateAccountBasedOnEmail());
}

@@ -336,8 +339,6 @@ public User getAuthor() {
public String getAuthorName() {
// If true, use the author field from git log rather than the committer.
String csAuthor = authorOrCommitter ? author : committer;
if (csAuthor == null)
throw new RuntimeException("No author in changeset " + id);
return csAuthor;
}

@@ -0,0 +1,96 @@
package hudson.plugins.git;

import java.util.ArrayList;
import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.*;

public class GitChangeSetEmptyTest {

private GitChangeSet changeSet = null;

public GitChangeSetEmptyTest() {
}

@Before
public void createEmptyChangeSet() {
changeSet = new GitChangeSet(new ArrayList<String>(), false);
}

@Test
public void testGetDate() {
assertNull(changeSet.getDate());
}

@Test
public void testGetCommitId() {
assertNull(changeSet.getCommitId());
}

@Test
public void testSetParent() {
changeSet.setParent(null);
assertNull(changeSet.getParent());
}

@Test
public void testGetParentCommit() {
assertNull(changeSet.getParentCommit());
}

@Test
public void testGetAffectedPaths() {
assertTrue(changeSet.getAffectedPaths().isEmpty());
}

@Test
public void testGetPaths() {
assertTrue(changeSet.getPaths().isEmpty());
}

@Test
public void testGetAffectedFiles() {
assertTrue(changeSet.getAffectedFiles().isEmpty());
}

@Test
public void testGetAuthorName() {
assertNull(changeSet.getAuthorName());
}

@Test
public void testGetMsg() {
assertNull(changeSet.getMsg());
}

@Test
public void testGetId() {
assertNull(changeSet.getId());
}

@Test
public void testGetRevision() {
assertNull(changeSet.getRevision());
}

@Test
public void testGetComment() {
assertNull(changeSet.getComment());
}

@Test
public void testGetBranch() {
assertNull(changeSet.getBranch());
}

@Test
public void testHashCode() {
assertTrue(changeSet.hashCode() != 0);
}

@Test
public void testEquals() {
assertFalse(changeSet.equals(null));
}

}
@@ -0,0 +1,146 @@
package hudson.plugins.git;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.*;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(Parameterized.class)
public class GitChangeSetEuroTest {

private GitChangeSet changeSet = null;
private final String id = "1567861636cd854f4dd6fa40bf94c0c657681dd5";
private final String parent = "e74a24e995305bd67a180f0ebc57927e2b8783ce";
private final String authorName = "Mr. Åhłañder";
private final String committerName = "Mister Åhländèr";
private final String msg = "[task] Updated version.";
private final String comment1 = "Including earlier updates.";
private final String commentStartText = msg + "\n\n" + comment1 + "\n";

protected final boolean useAuthorName;

public GitChangeSetEuroTest(String useAuthorName) {
this.useAuthorName = Boolean.valueOf(useAuthorName);
}

@Parameterized.Parameters(name = "{0}")
public static Collection permuteAuthorNameAndLegacyLayout() {
List<Object[]> values = new ArrayList<Object[]>();
String[] allowed = {"true", "false"};
for (String authorName : allowed) {
Object[] combination = {authorName};
values.add(combination);
}
return values;
}

@Before
public void createEuroChangeSet() {
ArrayList<String> gitChangeLog = new ArrayList<String>();
gitChangeLog.add("commit " + id);
gitChangeLog.add("tree 66236cf9a1ac0c589172b450ed01f019a5697c49");
gitChangeLog.add("parent " + parent);
gitChangeLog.add("author " + authorName + " <mister.ahlander@ericsson.com> 1363879004 +0100");
gitChangeLog.add("committer " + committerName + " <mister.ahlander@ericsson.com> 1364199539 -0400");
gitChangeLog.add("");
gitChangeLog.add(" " + msg);
gitChangeLog.add(" ");
gitChangeLog.add(" " + comment1);
gitChangeLog.add(" ");
gitChangeLog.add(" Changes in this version:");
gitChangeLog.add(" - Changed to take the gerrit url from gerrit query command.");
gitChangeLog.add(" - Aligned reason information with our new commit hooks");
gitChangeLog.add(" ");
gitChangeLog.add(" Change-Id: Ife96d2abed5b066d9620034bec5f04cf74b8c66d");
gitChangeLog.add(" Reviewed-on: https://gerrit.e.se/12345");
gitChangeLog.add(" Tested-by: Jenkins <jenkins@no-mail.com>");
gitChangeLog.add(" Reviewed-by: Mister Another <mister.another@ericsson.com>");
gitChangeLog.add("");
changeSet = new GitChangeSet(gitChangeLog, useAuthorName);
}

@Test
public void testGetDate() {
assertEquals(useAuthorName ? "2013-03-21T09:16:44-0600 +0100" : "2013-03-25T02:18:59-0600 -0400", changeSet.getDate());
}

@Test
public void testGetTimestamp() {
assertEquals(useAuthorName ? 1363879004000L : 1364199539000L, changeSet.getTimestamp());
}

@Test
public void testGetCommitId() {
assertEquals(id, changeSet.getCommitId());
}

@Test
public void testSetParent() {
changeSet.setParent(null);
assertNull(changeSet.getParent());
}

@Test
public void testGetParentCommit() {
assertEquals(parent, changeSet.getParentCommit());
}

@Test
public void testGetAffectedPaths() {
assertTrue(changeSet.getAffectedPaths().isEmpty());
}

@Test
public void testGetPaths() {
assertTrue(changeSet.getPaths().isEmpty());
}

@Test
public void testGetAffectedFiles() {
assertTrue(changeSet.getAffectedFiles().isEmpty());
}

@Test
public void testGetAuthorName() {
assertEquals(useAuthorName ? authorName : committerName, changeSet.getAuthorName());
}

@Test
public void testGetMsg() {
assertEquals(msg, changeSet.getMsg());
}

@Test
public void testGetId() {
assertEquals(id, changeSet.getId());
}

@Test
public void testGetRevision() {
assertEquals(id, changeSet.getRevision());
}

@Test
public void testGetComment() {
assertTrue(changeSet.getComment().startsWith(commentStartText));
}

@Test
public void testGetBranch() {
assertNull(changeSet.getBranch());
}

@Test
public void testHashCode() {
assertTrue(changeSet.hashCode() != 0);
}

@Test
public void testEquals() {
assertFalse(changeSet.equals(null));
}
}
Oops, something went wrong.

0 comments on commit 287e543

Please sign in to comment.