Skip to content

Commit

Permalink
[Fix JENKINS-16737] and [Fix JENKINS-10434] - no exception if author …
Browse files Browse the repository at this point in the history 
…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
MarkEWaite committed Oct 11, 2014
1 parent 6610db3 commit 287e543
Show file tree
Hide file tree
Showing 5 changed files with 502 additions and 60 deletions.
17 changes: 9 additions & 8 deletions src/main/java/hudson/plugins/git/GitChangeSet.java
View file
Expand Up @@ -247,6 +247,9 @@ public Collection<Path> getAffectedFiles() {
*/
public User findOrCreateUser(String csAuthor, String csAuthorEmail, boolean createAccountBasedOnEmail) {
User user;
if (csAuthor == null) {
return User.getUnknown();
}
if (createAccountBasedOnEmail) {
user = User.get(csAuthorEmail, false);

Expand Down Expand Up @@ -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();
}
Expand All @@ -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());
}

Expand All @@ -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;
}

Expand Down
96 changes: 96 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetEmptyTest.java
View file
@@ -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));
}

}
146 changes: 146 additions & 0 deletions src/test/java/hudson/plugins/git/GitChangeSetEuroTest.java
View file
@@ -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));
}
}

0 comments on commit 287e543

Please sign in to comment.