Skip to content

Commit

Permalink
Move JENKINS-33258 tests into CLI specific tests
Browse files Browse the repository at this point in the history 
The assertions are checking log messages that are only available from
CLI git.  No reason to spend time or energy calling methods that
won't generate the necessary logging output.
  • Loading branch information
@MarkEWaite
MarkEWaite committed Mar 20, 2020
1 parent 90a9f17 commit 3d498d1
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 29 deletions.
65 changes: 52 additions & 13 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitClientCliCloneTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.jenkinsci.plugins.gitclient;

import hudson.model.TaskListener;
import hudson.plugins.git.Branch;
import hudson.plugins.git.GitException;
import org.eclipse.jgit.transport.URIish;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -9,8 +11,11 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.Set;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;
Expand Down Expand Up @@ -45,6 +50,19 @@ public void setUpRepositories() throws Exception {
testGitClient = workspace.getGitClient();
}

/* JENKINS-33258 detected many calls to git rev-parse. This checks
* those calls are not being made. The checkoutRandomBranch call
* creates a branch with a random name. The later assertion checks that
* the random branch name is not mentioned in a call to git rev-parse.
*/
private String checkoutRandomBranch() throws GitException, InterruptedException {
String branchName = "rev-parse-branch-" + UUID.randomUUID().toString();
testGitClient.checkout().ref("origin/master").branch(branchName).execute();
Set<String> branchNames = testGitClient.getBranches().stream().map(Branch::getName).collect(Collectors.toSet());
assertThat(branchNames, hasItem(branchName));
return branchName;
}

@Test
public void test_clone_default_timeout_logging() throws Exception {
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").execute();
Expand All @@ -61,16 +79,20 @@ public void test_clone_timeout_logging() throws Exception {
@Test
public void test_fetch_default_timeout_logging() throws Exception {
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").execute();
testGitClient.fetch_().from(new URIish("origin"), null).execute();
String randomBranchName = checkoutRandomBranch();
testGitClient.fetch_().from(new URIish("origin"), null).prune(true).execute();
assertTimeout(testGitClient, "git fetch", CliGitAPIImpl.TIMEOUT);
assertRevParseNotCalled(testGitClient, randomBranchName);
}

@Test
public void test_fetch_timeout_logging() throws Exception {
int largerTimeout = CliGitAPIImpl.TIMEOUT + 1 + random.nextInt(600);
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").execute();
testGitClient.fetch_().from(new URIish("origin"), null).timeout(largerTimeout).execute();
String randomBranchName = checkoutRandomBranch(); // Check that prune(true) does not call git rev-parse
testGitClient.fetch_().from(new URIish("origin"), null).prune(true).timeout(largerTimeout).execute();
assertTimeout(testGitClient, "git fetch .* origin", largerTimeout);
assertRevParseNotCalled(testGitClient, randomBranchName);
}

@Test
Expand Down Expand Up @@ -98,24 +120,41 @@ public void test_submodule_update_timeout_logging() throws Exception {
assertTimeout(testGitClient, "git submodule update", largerTimeout);
}

protected void assertTimeout(GitClient gitClient, final String substring, int expectedTimeout) {
protected void assertLoggedMessage(GitClient gitClient, final String candidateSubstring, final String expectedValue, final boolean expectToFindMatch) {
List<String> messages = handler.getMessages();
List<String> substringMessages = new ArrayList<>();
List<String> substringTimeoutMessages = new ArrayList<>();
final String messageRegEx = ".*\\b" + substring + "\\b.*"; // the expected substring
final String timeoutRegEx = messageRegEx
+ " [#] timeout=" + expectedTimeout + "\\b.*"; // # timeout=<value>
List<String> candidateMessages = new ArrayList<>();
List<String> matchedMessages = new ArrayList<>();
final String messageRegEx = ".*\\b" + candidateSubstring + "\\b.*"; // the expected substring
final String timeoutRegEx = messageRegEx + expectedValue + "\\b.*"; // # timeout=<value>
for (String message : messages) {
if (message.matches(messageRegEx)) {
substringMessages.add(message);
candidateMessages.add(message);
}
if (message.matches(timeoutRegEx)) {
substringTimeoutMessages.add(message);
matchedMessages.add(message);
}
}
assertThat("No messages logged", messages, is(not(empty())));
assertThat("No messages matched substring '" + substring + "'", substringMessages, is(not(empty())));
assertThat("Messages matched substring '" + substring + "', found: " + substringMessages + "\nExpected timeout: " + expectedTimeout, substringTimeoutMessages, is(not(empty())));
assertThat("Timeout messages", substringTimeoutMessages, is(substringMessages));
if (expectToFindMatch) {
assertThat("No messages matched substring '" + candidateSubstring + "'", candidateMessages, is(not(empty())));
assertThat("Messages matched substring '" + candidateSubstring + "', found: " + candidateMessages + "\nExpected " + expectedValue, matchedMessages, is(not(empty())));
assertThat("All candidate messages matched", matchedMessages, is(candidateMessages));
} else {
assertThat("Messages matched substring '" + candidateSubstring + "' unexpectedly", candidateMessages, is(empty()));
}
}

private void assertTimeout(GitClient gitClient, final String substring, int expectedTimeout) {
assertLoggedMessage(gitClient, substring, " [#] timeout=" + expectedTimeout, true);
}

/* JENKINS-33258 detected many calls to git rev-parse. This checks
* those calls are not being made. The createRevParseBranch call
* creates a branch whose name is unknown to the tests. This
* checks that the branch name is not mentioned in a call to
* git rev-parse.
*/
private void assertRevParseNotCalled(GitClient gitClient, String unexpectedBranchName) {
assertLoggedMessage(gitClient, "git rev-parse ", unexpectedBranchName, false);
}
}
16 changes: 0 additions & 16 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitClientCloneTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.List;
import java.util.Random;
import java.util.Set;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand All @@ -45,9 +44,7 @@ public class GitClientCloneTest {

private int logCount = 0;
private final Random random = new Random();
private String revParseBranchName = null;
private LogHandler handler = null;
private static final String LOGGING_STARTED = "Logging started";
private TaskListener listener;
private final String gitImplName;

Expand Down Expand Up @@ -81,7 +78,6 @@ public void setUpRepositories() throws Exception {
logger.addHandler(handler);
logger.setLevel(Level.ALL);
listener = new hudson.util.LogTaskListener(logger, Level.ALL);
listener.getLogger().println(LOGGING_STARTED);

workspace = new WorkspaceWithRepo(repo.getRoot(), gitImplName, listener);
testGitClient = workspace.getGitClient();
Expand All @@ -107,7 +103,6 @@ public void test_clone() throws Exception {
}
cmd.execute();
assertSubstringTimeout(testGitClient, "git fetch", cloneTimeout);
createRevParseBranch(); // Verify JENKINS-32258 is fixed
testGitClient.checkout().ref("origin/master").branch("master").execute();
check_remote_url(workspace, testGitClient, "origin");
assertBranchesExist(testGitClient.getBranches(), "master");
Expand All @@ -122,7 +117,6 @@ public void test_checkout_exception() throws Exception {
cmd.noCheckout(); // Randomly confirm this deprecated call is a no-op
}
cmd.execute();
createRevParseBranch();
testGitClient.checkout().ref("origin/master").branch("master").execute();
final String SHA1 = "feedabeefabeadeddeedaccede";
GitException gitException = assertThrows(GitException.class, () -> {
Expand Down Expand Up @@ -151,7 +145,6 @@ public void test_clone_shallow() throws Exception {
cmd.noCheckout(); // Randomly confirm this deprecated call is a no-op
}
cmd.execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
testGitClient.checkout().ref("origin/master").branch("master").execute();
check_remote_url(workspace, testGitClient, "origin");
assertBranchesExist(testGitClient.getBranches(), "master");
Expand Down Expand Up @@ -188,7 +181,6 @@ public void test_clone_shallow_with_depth() throws Exception {
@Test
public void test_clone_shared() throws IOException, InterruptedException {
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").shared(true).execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
testGitClient.checkout().ref("origin/master").branch("master").execute();
check_remote_url(workspace, testGitClient, "origin");
assertBranchesExist(testGitClient.getBranches(), "master");
Expand All @@ -199,7 +191,6 @@ public void test_clone_shared() throws IOException, InterruptedException {
@Test
public void test_clone_null_branch() throws IOException, InterruptedException {
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").shared(true).execute();
createRevParseBranch();
testGitClient.checkout().ref("origin/master").branch(null).execute();
check_remote_url(workspace, testGitClient, "origin");
assertAlternateFilePointsToLocalMirror();
Expand All @@ -209,7 +200,6 @@ public void test_clone_null_branch() throws IOException, InterruptedException {
@Test
public void test_clone_unshared() throws IOException, InterruptedException {
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").shared(false).execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
testGitClient.checkout().ref("origin/master").branch("master").execute();
check_remote_url(workspace, testGitClient, "origin");
assertBranchesExist(testGitClient.getBranches(), "master");
Expand All @@ -219,7 +209,6 @@ public void test_clone_unshared() throws IOException, InterruptedException {
@Test
public void test_clone_reference() throws Exception, IOException, InterruptedException {
testGitClient.clone_().url(workspace.localMirror()).repositoryName("origin").reference(workspace.localMirror()).execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
testGitClient.checkout().ref("origin/master").branch("master").execute();
check_remote_url(workspace, testGitClient, "origin");
assertBranchesExist(testGitClient.getBranches(), "master");
Expand Down Expand Up @@ -348,11 +337,6 @@ private void assertAlternateFilePointsToLocalMirror() throws IOException, Interr
assertThat(alternatesDir, is(anExistingDirectory()));
}

protected void createRevParseBranch() throws GitException, InterruptedException {
revParseBranchName = "rev-parse-branch-" + UUID.randomUUID().toString();
testGitClient.checkout().ref("origin/master").branch(revParseBranchName).execute();
}

private Collection<String> getBranchNames(Collection<Branch> branches) {
return branches.stream().map(Branch::getName).collect(toList());
}
Expand Down

0 comments on commit 3d498d1

Please sign in to comment.