Skip to content

Commit

Permalink
minor test refactoring around StandardCharsets, simplified JUnit asse…
Browse files Browse the repository at this point in the history
…rtions, removed obsolete throws, fixed deprecation warnings (#4872)
  • Loading branch information
StefanSpieker committed Aug 11, 2020
1 parent 264a2d5 commit f91b1f7
Show file tree
Hide file tree
Showing 14 changed files with 108 additions and 105 deletions.
16 changes: 7 additions & 9 deletions test/src/test/java/hudson/PluginManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@
import java.lang.reflect.Method;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.Future;

import jenkins.ClassLoaderReflectionToolkit;
Expand Down Expand Up @@ -118,15 +116,15 @@ public class PluginManagerTest {
* Tests the effect of {@link WithPlugin}.
*/
@WithPlugin("tasks.jpi")
@Test public void withRecipeJpi() throws Exception {
@Test public void withRecipeJpi() {
assertNotNull(r.jenkins.getPlugin("tasks"));
}

/**
* Tests the effect of {@link WithPlugin}.
*/
@WithPlugin("legacy.hpi")
@Test public void withRecipeHpi() throws Exception {
@Test public void withRecipeHpi() {
assertNotNull(r.jenkins.getPlugin("legacy"));
}

Expand Down Expand Up @@ -178,7 +176,7 @@ public void startPlugin(PluginWrapper plugin) throws Exception {
tested = true;

// plugins should be already visible in the UberClassLoader
assertTrue(!activePlugins.isEmpty());
assertFalse(activePlugins.isEmpty());

uberClassLoader.loadClass("hudson.plugins.tasks.Messages");

Expand Down Expand Up @@ -485,9 +483,9 @@ private String callDependerValue() throws Exception {
// Check that there was some data in the response and that the first entry
// at least had some of the expected fields.
JSONObject pluginInfo = data.getJSONObject(0);
assertTrue(pluginInfo.getString("name") != null);
assertTrue(pluginInfo.getString("title") != null);
assertTrue(pluginInfo.getString("dependencies") != null);
assertNotNull(pluginInfo.getString("name"));
assertNotNull(pluginInfo.getString("title"));
assertNotNull(pluginInfo.getString("dependencies"));
}

@Issue("JENKINS-41684")
Expand Down Expand Up @@ -681,7 +679,7 @@ public void doNotThrowWithUnknownPlugins() throws Exception {
Assert.assertNull("This test requires the plugin with ID 'legacy' to not exist in update sites", uc.getPlugin("legacy"));

// ensure data is loaded - probably unnecessary, but closer to reality
Assert.assertTrue(uc.getSite("default").updateDirectlyNow().kind == FormValidation.Kind.OK);
Assert.assertSame(uc.getSite("default").updateDirectlyNow().kind, FormValidation.Kind.OK);

// This would throw NPE
uc.getPluginsWithUnavailableUpdates();
Expand Down
10 changes: 6 additions & 4 deletions test/src/test/java/hudson/cli/CreateJobCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import hudson.model.Item;
import hudson.model.User;
import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;

import jenkins.model.Jenkins;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertNotNull;
Expand All @@ -53,10 +55,10 @@ public class CreateJobCommandTest {
grant(Jenkins.READ).everywhere().toAuthenticated().
grant(Item.READ).onItems(d).toAuthenticated(). // including alice
grant(Item.CREATE).onItems(d).to("bob"));
cmd.setTransportAuth(User.get("alice").impersonate());
assertThat(invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes("US-ASCII"))).invokeWithArgs("d/p"), failedWith(6));
cmd.setTransportAuth(User.get("bob").impersonate());
assertThat(invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes("US-ASCII"))).invokeWithArgs("d/p"), succeededSilently());
cmd.setTransportAuth(User.getOrCreateByIdOrFullName("alice").impersonate());
assertThat(invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes(StandardCharsets.UTF_8))).invokeWithArgs("d/p"), failedWith(6));
cmd.setTransportAuth(User.getOrCreateByIdOrFullName("bob").impersonate());
assertThat(invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes(StandardCharsets.UTF_8))).invokeWithArgs("d/p"), succeededSilently());
assertNotNull(d.getItem("p"));
}

Expand Down
80 changes: 40 additions & 40 deletions test/src/test/java/hudson/cli/RunRangeCommandTest.java

Large diffs are not rendered by default.

11 changes: 6 additions & 5 deletions test/src/test/java/hudson/model/ItemsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.io.File;
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import jenkins.model.Jenkins;
import jenkins.security.apitoken.ApiTokenTestHelper;
Expand Down Expand Up @@ -291,8 +292,8 @@ private enum OverwriteTactic {
@Override void run(JenkinsRule r, String target) throws Exception {
CLICommand cmd = new CreateJobCommand();
CLICommandInvoker invoker = new CLICommandInvoker(r, cmd);
cmd.setTransportAuth(User.get("attacker").impersonate());
int status = invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes("US-ASCII"))).invokeWithArgs(target).returnCode();
cmd.setTransportAuth(User.getOrCreateByIdOrFullName("attacker").impersonate());
int status = invoker.withStdin(new ByteArrayInputStream("<project/>".getBytes(StandardCharsets.UTF_8))).invokeWithArgs(target).returnCode();
if (status != 0) {
throw new AbortException("CLI command failed with status " + status);
}
Expand All @@ -304,7 +305,7 @@ private enum OverwriteTactic {
r.createFreeStyleProject("dupe");
CLICommand cmd = new CopyJobCommand();
CLICommandInvoker invoker = new CLICommandInvoker(r, cmd);
cmd.setTransportAuth(User.get("attacker").impersonate());
cmd.setTransportAuth(User.getOrCreateByIdOrFullName("attacker").impersonate());
int status = invoker.invokeWithArgs("dupe", target).returnCode();
r.jenkins.getItem("dupe").delete();
if (status != 0) {
Expand All @@ -316,7 +317,7 @@ private enum OverwriteTactic {
MOVE {
@Override void run(JenkinsRule r, String target) throws Exception {
try {
SecurityContext orig = ACL.impersonate(User.get("attacker").impersonate());
SecurityContext orig = ACL.impersonate(User.getOrCreateByIdOrFullName("attacker").impersonate());
try {
Items.move(r.jenkins.getItemByFullName("d", MockFolder.class).createProject(FreeStyleProject.class, target), r.jenkins);
} finally {
Expand All @@ -330,7 +331,7 @@ private enum OverwriteTactic {
}
};
abstract void run(JenkinsRule r, String target) throws Exception;
private static final JenkinsRule.WebClient wc(JenkinsRule r) {
private static JenkinsRule.WebClient wc(JenkinsRule r) {
return r.createWebClient().withBasicApiToken("attacker");
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/src/test/java/hudson/model/JobTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ private String dirContent(File dir) throws IOException, InterruptedException {
if (dir == null || !dir.isDirectory()) {
return null;
}
StringBuilder str = new StringBuilder("");
StringBuilder str = new StringBuilder();
final FilePath[] list = new FilePath(dir).list("**/*");
Arrays.sort(list, Comparator.comparing(FilePath::getRemote));
for (FilePath path : list) {
Expand Down
20 changes: 10 additions & 10 deletions test/src/test/java/hudson/model/ProjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ public void testSave() throws IOException, InterruptedException, ReactorExceptio
j.jenkins.reload();
assertEquals("All persistent data should be saved.", "description", p.description);
assertEquals("All persistent data should be saved.", 5, p.nextBuildNumber);
assertEquals("All persistent data should be saved", true, p.disabled);
assertTrue("All persistent data should be saved", p.disabled);
}

@Test
public void testOnCreateFromScratch() throws IOException, Exception{
public void testOnCreateFromScratch() throws Exception{
FreeStyleProject p = j.createFreeStyleProject("project");
j.buildAndAssertSuccess(p);
p.removeRun(p.getLastBuild());
Expand All @@ -135,15 +135,15 @@ public void testOnCreateFromScratch() throws IOException, Exception{
}

@Test
public void testOnLoad() throws IOException, Exception{
public void testOnLoad() throws Exception{
FreeStyleProject p = j.createFreeStyleProject("project");
j.buildAndAssertSuccess(p);
p.removeRun(p.getLastBuild());
createAction = true;
p.onLoad(j.jenkins, "project");
assertTrue("Project should have a build.", p.getLastBuild()!=null);
assertTrue("Project should have a scm.", p.getScm()!=null);
assertTrue("Project should have Transient Action TransientAction.", p.getAction(TransientAction.class)!=null);
assertNotNull("Project should have a build.", p.getLastBuild());
assertNotNull("Project should have a scm.", p.getScm());
assertNotNull("Project should have Transient Action TransientAction.", p.getAction(TransientAction.class));
createAction = false;
}

Expand All @@ -158,7 +158,7 @@ public void testGetEnvironment() throws Exception{
}

@Test
public void testPerformDelete() throws IOException, Exception{
public void testPerformDelete() throws Exception{
FreeStyleProject p = j.createFreeStyleProject("project");
p.performDelete();
assertFalse("Project should be deleted from disk.", p.getConfigFile().exists());
Expand Down Expand Up @@ -328,7 +328,7 @@ public void testSchedulePolling() throws IOException, ANTLRException{
}

@Test
public void testSaveAfterSet() throws Exception, ReactorException {
public void testSaveAfterSet() throws Exception {
FreeStyleProject p = j.createFreeStyleProject("project");
p.setScm(new NullSCM());
p.setScmCheckoutStrategy(new SCMCheckoutStrategyImpl());
Expand Down Expand Up @@ -439,7 +439,7 @@ public void testCreateExecutable() throws IOException{
}

@Test
public void testCheckout() throws IOException, Exception{
public void testCheckout() throws Exception{
SCM scm = new NullSCM();
FreeStyleProject p = j.createFreeStyleProject("project");
Slave slave = j.createOnlineSlave();
Expand Down Expand Up @@ -952,7 +952,7 @@ public static class SubTaskImpl implements SubTask{
public String projectName;

@Override
public Executable createExecutable() throws IOException {
public Executable createExecutable() {
return null;
}

Expand Down
9 changes: 5 additions & 4 deletions test/src/test/java/hudson/model/SlaveTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.HashSet;
import java.util.Set;
import edu.umd.cs.findbugs.annotations.CheckForNull;
Expand Down Expand Up @@ -110,7 +111,7 @@ private void post(String url, String xml) throws Exception {
con.setRequestProperty("Content-Type", "application/xml;charset=UTF-8");
con.setRequestProperty(CrumbIssuer.DEFAULT_CRUMB_NAME, "test");
con.setDoOutput(true);
con.getOutputStream().write(xml.getBytes("UTF-8"));
con.getOutputStream().write(xml.getBytes(StandardCharsets.UTF_8));
con.getOutputStream().close();
IOUtils.copy(con.getInputStream(), System.out);
}
Expand Down Expand Up @@ -181,7 +182,7 @@ private void assertJnlpJarUrlIsAllowed(@NonNull Slave slave, @NonNull String url

@Test
@Issue("JENKINS-36280")
public void launcherFiltering() throws Exception {
public void launcherFiltering() {
DumbSlave.DescriptorImpl descriptor =
j.getInstance().getDescriptorByType(DumbSlave.DescriptorImpl.class);
DescriptorExtensionList<ComputerLauncher, Descriptor<ComputerLauncher>> descriptors =
Expand All @@ -199,7 +200,7 @@ public void launcherFiltering() throws Exception {

@Test
@Issue("JENKINS-36280")
public void retentionFiltering() throws Exception {
public void retentionFiltering() {
DumbSlave.DescriptorImpl descriptor =
j.getInstance().getDescriptorByType(DumbSlave.DescriptorImpl.class);
DescriptorExtensionList<RetentionStrategy<?>, Descriptor<RetentionStrategy<?>>> descriptors = RetentionStrategy.all();
Expand All @@ -216,7 +217,7 @@ public void retentionFiltering() throws Exception {

@Test
@Issue("JENKINS-36280")
public void propertyFiltering() throws Exception {
public void propertyFiltering() {
j.jenkins.setAuthorizationStrategy(new ProjectMatrixAuthorizationStrategy()); // otherwise node descriptor is not available
DumbSlave.DescriptorImpl descriptor =
j.getInstance().getDescriptorByType(DumbSlave.DescriptorImpl.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void testExceptionOnNodeProperty() throws Exception {
assertThat(r.getInstance().getQueue().getBuildableItems().get(0).task.getName(), equalTo("theFaultyOne"));

// The new error is shown in the logs
assertThat(logging.getMessages(), hasItem(String.format("Exception evaluating if the node '%s' can take the task '%s'", new Object[]{faultyAgent.getDisplayName(), "theFaultyOne"})));
assertThat(logging.getMessages(), hasItem(String.format("Exception evaluating if the node '%s' can take the task '%s'", faultyAgent.getDisplayName(), "theFaultyOne")));
}

/**
Expand Down
12 changes: 7 additions & 5 deletions test/src/test/java/hudson/security/ACLTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package hudson.security;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.model.Build;
import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import hudson.model.FreeStyleProject;
Expand Down Expand Up @@ -70,7 +71,7 @@ public void bypassStrategyOnSystem() throws Exception {
}

@Test
public void checkAnyPermissionPassedIfOneIsValid() throws Exception {
public void checkAnyPermissionPassedIfOneIsValid() {
Jenkins jenkins = r.jenkins;
jenkins.setSecurityRealm(r.createDummySecurityRealm());
jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
Expand All @@ -84,7 +85,7 @@ public void checkAnyPermissionPassedIfOneIsValid() throws Exception {
}

@Test
public void checkAnyPermissionThrowsIfPermissionIsMissing() throws Exception {
public void checkAnyPermissionThrowsIfPermissionIsMissing() {
Jenkins jenkins = r.jenkins;
jenkins.setSecurityRealm(r.createDummySecurityRealm());
jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
Expand All @@ -101,7 +102,7 @@ public void checkAnyPermissionThrowsIfPermissionIsMissing() throws Exception {
}

@Test
public void checkAnyPermissionThrowsIfMissingMoreThanOne() throws Exception {
public void checkAnyPermissionThrowsIfMissingMoreThanOne() {
Jenkins jenkins = r.jenkins;
jenkins.setSecurityRealm(r.createDummySecurityRealm());
jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
Expand All @@ -119,7 +120,7 @@ public void checkAnyPermissionThrowsIfMissingMoreThanOne() throws Exception {

@Test
@Issue("JENKINS-61467")
public void checkAnyPermissionDoesNotShowDisabledPermissionsInError() throws Exception {
public void checkAnyPermissionDoesNotShowDisabledPermissionsInError() {
Jenkins jenkins = r.jenkins;
jenkins.setSecurityRealm(r.createDummySecurityRealm());
jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
Expand All @@ -137,7 +138,7 @@ public void checkAnyPermissionDoesNotShowDisabledPermissionsInError() throws Exc

@Test
@Issue("JENKINS-61467")
public void checkAnyPermissionShouldShowDisabledPermissionsIfNotImplied() throws Exception {
public void checkAnyPermissionShouldShowDisabledPermissionsIfNotImplied() {
Jenkins jenkins = r.jenkins;
jenkins.setSecurityRealm(r.createDummySecurityRealm());
jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
Expand Down Expand Up @@ -200,6 +201,7 @@ public boolean hasPermission(Authentication a, Permission permission) {
};
}

@NonNull
@Override
public Collection<String> getGroups() {
return Collections.emptySet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,11 @@
import com.gargoylesoftware.htmlunit.xml.XmlPage;
import hudson.ExtensionList;
import hudson.model.User;
import hudson.remoting.Base64;
import static hudson.security.HudsonPrivateSecurityRealm.CLASSIC;
import static hudson.security.HudsonPrivateSecurityRealm.PASSWORD_ENCODER;
import hudson.security.pages.SignupPage;
import java.io.UnsupportedEncodingException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -87,7 +86,7 @@ public class HudsonPrivateSecurityRealmTest {
private SpySecurityListenerImpl spySecurityListener;

@Before
public void linkExtension() throws Exception {
public void linkExtension() {
spySecurityListener = ExtensionList.lookup(SecurityListener.class).get(SpySecurityListenerImpl.class);
}

Expand Down Expand Up @@ -203,11 +202,10 @@ public void fullNameCollisionToken() throws Exception {
}


private static final String basicHeader(String user, String pass) throws UnsupportedEncodingException {
private static String basicHeader(String user, String pass) {
String str = user +':' + pass;
String auth = Base64.encode(str.getBytes("US-ASCII"));
String authHeader = "Basic " + auth;
return authHeader;
String auth = java.util.Base64.getEncoder().encodeToString(str.getBytes(StandardCharsets.UTF_8));
return "Basic " + auth;
}

@Test
Expand Down Expand Up @@ -426,8 +424,8 @@ private void selfRegistration(String login) throws Exception {

@TestExtension
public static class SpySecurityListenerImpl extends SecurityListener {
private List<String> loggedInUsernames = new ArrayList<>();
private List<String> createdUsers = new ArrayList<String>();
private final List<String> loggedInUsernames = new ArrayList<>();
private final List<String> createdUsers = new ArrayList<>();

@Override
protected void loggedIn(@NonNull String username) {
Expand Down Expand Up @@ -471,7 +469,6 @@ public void controlCharacterAreNoMoreValid() throws Exception {
checkUserCannotBeCreatedWith(securityRealm, "Stargåte" + i, password, "Test" + i, email);
i++;
checkUserCannotBeCreatedWith(securityRealm, "te\u0000st" + i, password, "Test" + i, email);
i++;
}
}

Expand Down Expand Up @@ -514,7 +511,6 @@ public void controlCharacterAreNoMoreValid_CustomRegex() throws Exception {
assertNotNull(User.getById("125213" + i, false));
i++;
checkUserCannotBeCreatedWith_custom(securityRealm, "TEST12" + i, password, "Test" + i, email, currentRegex);
i++;
}
}

Expand Down

0 comments on commit f91b1f7

Please sign in to comment.