Skip to content

Commit

Permalink
Changes were made and a similar test without failing is transferred f…
Browse files Browse the repository at this point in the history 
…rom GitAPITestCase to GitClientTest.
  • Loading branch information
@loghijiaha
loghijiaha committed Feb 12, 2020
1 parent 7033fdb commit fe3dfe4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 25 deletions.
22 changes: 0 additions & 22 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4064,28 +4064,6 @@ public void test_checkout_interrupted_with_existing_lock() throws Exception {
assertTrue("lock file '" + lockFile.getCanonicalPath() + " removed by cleanup", lockFile.exists());
}

/**
* Test case for auto local branch creation behviour.
* This is essentially a stripped down version of {@link #test_branchContainingRemote()}
* @throws Exception on exceptions occur
*/
public void test_checkout_remote_autocreates_local() throws Exception {
final WorkingArea r = new WorkingArea();
r.init();
r.commitEmpty("c1");

w.git.clone_().url("file://" + r.repoPath()).execute();
final URIish remote = new URIish(Constants.DEFAULT_REMOTE_NAME);
final List<RefSpec> refspecs = Collections.singletonList(new RefSpec(
"refs/heads/*:refs/remotes/origin/*"));
w.git.fetch_().from(remote, refspecs).execute();
w.git.checkout().ref(Constants.MASTER).execute();

Set<String> refNames = w.git.getRefNames("refs/heads/");
assertThat(refNames, contains("refs/heads/master"));
}


public void test_revList_remote_branch() throws Exception {
w = clone(localMirror());
List<ObjectId> revList = w.git.revList("origin/1.4.x");
Expand Down
35 changes: 32 additions & 3 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ public void testAddRemoteUrl() throws Exception {
assertEquals(upstreamRepoURL, gitClient.getRemoteUrl("upstream"));
}

@Test (expected = GitException.class)
@Test
public void testAutocreateFailsOnMultipleMatchingOrigins() throws Exception {
File repoRootTemp = tempFolder.newFolder();
GitClient gitClientTemp = Git.with(TaskListener.NULL, new EnvVars()).in(repoRootTemp).using(gitImplName).getClient();
Expand Down Expand Up @@ -629,10 +629,39 @@ public void testAutocreateFailsOnMultipleMatchingOrigins() throws Exception {
gitClient.fetch_().from(remote, refspecs).execute();

// checkout will fail
try {
gitClient.checkout().ref(Constants.MASTER).execute();

This comment has been minimized.

Sign in to view

Copy link
@darxriggs

darxriggs Feb 17, 2020

Contributor

There should be sth. like a fail("Did not throw expected exception") call here, like in many other tests.
Otherwise the test is successful even if no exception was thrown at all.

This comment has been minimized.

Sign in to view

Copy link
@loghijiaha

loghijiaha Feb 18, 2020

Author Contributor

As @MarkEWaite suggested, the fail("Did not throw expected exception") was too broad. Then I added assertFalse("RefNames will not contain master", refNames.contains("refs/heads/master")) for a specific reason why the checkout fails.

This comment has been minimized.

Sign in to view

Copy link
@darxriggs

darxriggs Apr 7, 2020

Contributor

Ok let me try to rephrase it. What is the test result now and what has it been before this change if the code under test (line 633) would not throw an exception? This path should be covered in the test too. @Test(expected = ...) is handling this in a better way than the current test implementation, that was my point. This JUnit documentation https://github.com/junit-team/junit4/wiki/Exception-testing also describes the possibilities.

This comment has been minimized.

Sign in to view

Copy link
@MarkEWaite

MarkEWaite Apr 7, 2020

Contributor

JUnit 4.13 provides a very nice way to assert that exceptions are thrown using assertThrows. It is now used in a few places in the plugin tests and is becoming more comfortable for me as more and more tests use it.
} catch (GitException e) {
// expected
Set<String> refNames = gitClient.getRefNames("refs/heads/");
assertFalse("RefNames will not contain master", refNames.contains("refs/heads/master"));
}

}

/**
* Test case for auto local branch creation behviour.
* This is essentially a stripped down version of {@link GitAPITestCase#test_branchContainingRemote()}
* @throws Exception on exceptions occur
*/
@Test
public void testCheckoutRemoteAutocreatesLocal() throws Exception {
File repoRootTemp = tempFolder.newFolder();
GitClient gitClientTemp = Git.with(TaskListener.NULL, new EnvVars()).in(repoRootTemp).using(gitImplName).getClient();
gitClientTemp.init();
FilePath gitClientFilePath = gitClientTemp.getWorkTree();
FilePath gitClientTempFile = gitClientFilePath.createTextTempFile("aPre", ".txt", "file contents");
gitClientTemp.add(".");
gitClientTemp.commit("Added " + gitClientTempFile.toURI().toString());
gitClient.clone_().url("file://" + repoRootTemp.getPath()).execute();
final URIish remote = new URIish(Constants.DEFAULT_REMOTE_NAME);
final List<RefSpec> refspecs = Collections.singletonList(new RefSpec(
"refs/heads/*:refs/remotes/origin/*"));
gitClient.fetch_().from(remote, refspecs).execute();
gitClient.checkout().ref(Constants.MASTER).execute();
Set<String> refNames = gitClient.getRefNames("refs/heads/");
assertFalse("RefNames will not contain master", refNames.contains("refs/heads/master"));

Set<String> refNames = gitClient.getRefNames("refs/heads/");
assertThat(refNames, contains("refs/heads/master"));
}

private void assertFileInWorkingDir(GitClient client, String fileName) {
Expand Down

0 comments on commit fe3dfe4

Please sign in to comment.