Skip to content

Commit

Permalink
Fix erratic GitStatusTest (jenkinsci#1480)
Browse files Browse the repository at this point in the history 
* Fix random GitStatusTest failures

Test already had code that waits for jobs to complete on Windows.
The improved job leak detection that was implemented in the Jenkins
test harness will sometimes show the same conditions on Unix builds.
Wait for jobs to complete on Windows and on Unix.

* Remove non JenkinsRule tests to simplify cleanup

* Move simple GitStatusTest items to new class

No need to complicate GitStatusTest with the test methods that do not
require a JenkinsRule.  Use a simpler test structure and a simpler test
file for the GitStatusTest assertions that do not require a JenkinsRule.

* Assert that the "all" view must exist

* Report a message if no jobs found for wait loop

Not an error, but surprising based on my testing.  None of my test
machines are fast enough to complete all the jobs without needing any
time for waiting.

* Replace Hudson with Jenkins in comments

* Remove unused imports
  • Loading branch information
@MarkEWaite
MarkEWaite committed Jul 8, 2023
1 parent 3b561eb commit fd87a1c
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 82 deletions.
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/git/GitChangeSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ public User getAuthor() {
/**
* Gets the author name for this changeset - note that this is mainly here
* so that we can test authorOrCommitter without needing a fully instantiated
* Hudson (which is needed for User.get in getAuthor()).
* Jenkins (which is needed for User.get in getAuthor()).
*
* @return author name
*/
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/git/GitSCM.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ public PollingResult compareRemoteRevisionWith(Job<?, ?> project, Launcher launc
public static final Pattern GIT_REF = Pattern.compile("^(refs/[^/]+)/(.+)");

private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher launcher, FilePath workspace, final @NonNull TaskListener listener) throws IOException, InterruptedException {
// Poll for changes. Are there any unbuilt revisions that Hudson ought to build ?
// Poll for changes. Are there any unbuilt revisions that Jenkins ought to build ?

listener.getLogger().println("Using strategy: " + getBuildChooser().getDisplayName());

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/git/GitStatus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.kohsuke.stapler.*;

/**
* Information screen for the use of Git in Hudson.
* Root action that requests the plugin to poll for changes in remote repositories.
*/
@Extension
public class GitStatus implements UnprotectedRootAction {
Expand Down
86 changes: 86 additions & 0 deletions src/test/java/hudson/plugins/git/GitStatusSimpleTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package hudson.plugins.git;

import static org.junit.Assert.*;

import org.eclipse.jgit.transport.URIish;
import java.util.ArrayList;
import java.util.List;
import org.junit.Before;
import org.junit.Test;

public class GitStatusSimpleTest {

private GitStatus gitStatus;

@Before
public void setUp() throws Exception {
this.gitStatus = new GitStatus();
}

@Test
public void testGetDisplayName() {
assertEquals("Git", this.gitStatus.getDisplayName());
}

@Test
public void testGetIconFileName() {
assertNull(this.gitStatus.getIconFileName());
}

@Test
public void testGetUrlName() {
assertEquals("git", this.gitStatus.getUrlName());
}

@Test
public void testAllowNotifyCommitParametersDisabled() {
assertFalse("SECURITY-275: ignore arbitrary notifyCommit parameters", GitStatus.ALLOW_NOTIFY_COMMIT_PARAMETERS);
}

@Test
public void testSafeParametersEmpty() {
assertEquals("SECURITY-275: Safe notifyCommit parameters", "", GitStatus.SAFE_PARAMETERS);
}

@Test
public void testLooselyMatches() throws Exception {
String[] equivalentRepoURLs = new String[] {
"https://example.com/jenkinsci/git-plugin",
"https://example.com/jenkinsci/git-plugin/",
"https://example.com/jenkinsci/git-plugin.git",
"https://example.com/jenkinsci/git-plugin.git/",
"https://[email protected]/jenkinsci/git-plugin.git",
"https://someone:[email protected]/jenkinsci/git-plugin/",
"git://example.com/jenkinsci/git-plugin",
"git://example.com/jenkinsci/git-plugin/",
"git://example.com/jenkinsci/git-plugin.git",
"git://example.com/jenkinsci/git-plugin.git/",
"ssh://[email protected]/jenkinsci/git-plugin",
"ssh://example.com/jenkinsci/git-plugin.git",
"[email protected]:jenkinsci/git-plugin/",
"[email protected]:jenkinsci/git-plugin.git",
"[email protected]:jenkinsci/git-plugin.git/"
};
List<URIish> uris = new ArrayList<>();
for (String testURL : equivalentRepoURLs) {
uris.add(new URIish(testURL));
}

/* Extra slashes on end of URL probably should be considered equivalent,
* but current implementation does not consider them as loose matches
*/
URIish badURLTrailingSlashes = new URIish(equivalentRepoURLs[0] + "///");
/* Different hostname should always fail match check */
URIish badURLHostname = new URIish(equivalentRepoURLs[0].replace("example.com", "bitbucket.org"));

for (URIish lhs : uris) {
assertFalse(
lhs + " matches trailing slashes " + badURLTrailingSlashes,
GitStatus.looselyMatches(lhs, badURLTrailingSlashes));
assertFalse(lhs + " matches bad hostname " + badURLHostname, GitStatus.looselyMatches(lhs, badURLHostname));
for (URIish rhs : uris) {
assertTrue(lhs + " and " + rhs + " didn't match", GitStatus.looselyMatches(lhs, rhs));
}
}
}
}
83 changes: 4 additions & 79 deletions src/test/java/hudson/plugins/git/GitStatusTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.WithoutJenkins;

import javax.servlet.http.HttpServletRequest;
import org.kohsuke.stapler.HttpResponse;
Expand Down Expand Up @@ -72,10 +71,8 @@ public void resetAllowNotifyCommitParameters() throws Exception {

@After
public void waitForAllJobsToComplete() throws Exception {
// Put JenkinsRule into shutdown state, trying to reduce Windows cleanup exceptions
if (r != null && r.jenkins != null) {
r.jenkins.doQuietDown();
}
// Put JenkinsRule into shutdown state, trying to reduce cleanup exceptions
r.jenkins.doQuietDown();
// JenkinsRule cleanup throws exceptions during tearDown.
// Reduce exceptions by a random delay from 0.5 to 0.9 seconds.
// Adding roughly 0.7 seconds to these JenkinsRule tests is a small price
Expand All @@ -87,15 +84,14 @@ public void waitForAllJobsToComplete() throws Exception {
* build logs will not be active when the cleanup process tries to
* delete them.
*/
if (!isWindows() || r == null || r.jenkins == null) {
return;
}
View allView = r.jenkins.getView("All");
if (allView == null) {
fail("All view was not found when it should always be available");
return;
}
RunList<Run> runList = allView.getBuilds();
if (runList == null) {
Logger.getLogger(GitStatusTest.class.getName()).log(Level.INFO, "No waiting, no entries in the runList");
return;
}
runList.forEach((Run run) -> {
Expand All @@ -108,36 +104,6 @@ public void waitForAllJobsToComplete() throws Exception {
});
}

@WithoutJenkins
@Test
public void testGetDisplayName() {
assertEquals("Git", this.gitStatus.getDisplayName());
}

@WithoutJenkins
@Test
public void testGetIconFileName() {
assertNull(this.gitStatus.getIconFileName());
}

@WithoutJenkins
@Test
public void testGetUrlName() {
assertEquals("git", this.gitStatus.getUrlName());
}

@WithoutJenkins
@Test
public void testAllowNotifyCommitParametersDisabled() {
assertFalse("SECURITY-275: ignore arbitrary notifyCommit parameters", GitStatus.ALLOW_NOTIFY_COMMIT_PARAMETERS);
}

@WithoutJenkins
@Test
public void testSafeParametersEmpty() {
assertEquals("SECURITY-275: Safe notifyCommit parameters", "", GitStatus.SAFE_PARAMETERS);
}

@Test
public void testDoNotifyCommitWithNoBranches() throws Exception {
SCMTrigger aMasterTrigger = setupProjectWithTrigger("a", "master", false);
Expand Down Expand Up @@ -371,47 +337,6 @@ private void setupProject(String url, String branchString, SCMTrigger trigger) t
if (trigger != null) project.addTrigger(trigger);
}

@WithoutJenkins
@Test
public void testLooselyMatches() throws URISyntaxException {
String[] equivalentRepoURLs = new String[]{
"https://example.com/jenkinsci/git-plugin",
"https://example.com/jenkinsci/git-plugin/",
"https://example.com/jenkinsci/git-plugin.git",
"https://example.com/jenkinsci/git-plugin.git/",
"https://[email protected]/jenkinsci/git-plugin.git",
"https://someone:[email protected]/jenkinsci/git-plugin/",
"git://example.com/jenkinsci/git-plugin",
"git://example.com/jenkinsci/git-plugin/",
"git://example.com/jenkinsci/git-plugin.git",
"git://example.com/jenkinsci/git-plugin.git/",
"ssh://[email protected]/jenkinsci/git-plugin",
"ssh://example.com/jenkinsci/git-plugin.git",
"[email protected]:jenkinsci/git-plugin/",
"[email protected]:jenkinsci/git-plugin.git",
"[email protected]:jenkinsci/git-plugin.git/"
};
List<URIish> uris = new ArrayList<>();
for (String testURL : equivalentRepoURLs) {
uris.add(new URIish(testURL));
}

/* Extra slashes on end of URL probably should be considered equivalent,
* but current implementation does not consider them as loose matches
*/
URIish badURLTrailingSlashes = new URIish(equivalentRepoURLs[0] + "///");
/* Different hostname should always fail match check */
URIish badURLHostname = new URIish(equivalentRepoURLs[0].replace("example.com", "bitbucket.org"));

for (URIish lhs : uris) {
assertFalse(lhs + " matches trailing slashes " + badURLTrailingSlashes, GitStatus.looselyMatches(lhs, badURLTrailingSlashes));
assertFalse(lhs + " matches bad hostname " + badURLHostname, GitStatus.looselyMatches(lhs, badURLHostname));
for (URIish rhs : uris) {
assertTrue(lhs + " and " + rhs + " didn't match", GitStatus.looselyMatches(lhs, rhs));
}
}
}

private FreeStyleProject setupNotifyProject() throws Exception {
FreeStyleProject project = r.createFreeStyleProject();
project.setQuietPeriod(0);
Expand Down

0 comments on commit fd87a1c

Please sign in to comment.