Fix errorprone warnings (UnusedVariable, out of order params)

dspace-api/src/test/java/org/dspace/content/BitstreamFormatTest.java

@@ -232,7 +232,7 @@ public class BitstreamFormatTest extends AbstractUnitTest {
         // Disalow full Admin perms
         when(authorizeServiceSpy.isAdmin(context)).thenReturn(false);
 
-        BitstreamFormat found = bitstreamFormatService.create(context);
+        bitstreamFormatService.create(context);
         fail("Exception should have been thrown");
     }
 

dspace-api/src/test/java/org/dspace/content/CommunityTest.java

@@ -195,7 +195,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
 
         // test creating community with no parent (as a non-admin)
         // this should throw an exception
-        Community created = communityService.create(null, context);
+        communityService.create(null, context);
         fail("Exception expected");
     }
 
@@ -230,7 +230,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
 
         // test creating community with a specified handle which IS already in use
         // This should throw an exception
-        Community created = communityService.create(null, context, inUseHandle);
+        communityService.create(null, context, inUseHandle);
         fail("Exception expected");
     }
 
@@ -378,7 +378,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, c, Constants.WRITE);
 
         File f = new File(testProps.get("test.bitstream").toString());
-        Bitstream logo = communityService.setLogo(context, c, new FileInputStream(f));
+        communityService.setLogo(context, c, new FileInputStream(f));
         fail("Exception expected");
     }
 
@@ -425,7 +425,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
      */
     @Test(expected = AuthorizeException.class)
     public void testCreateAdministratorsNoAuth() throws Exception {
-        Group result = communityService.createAdministrators(context, c);
+        communityService.createAdministrators(context, c);
         fail("Exception should have been thrown");
     }
 
@@ -617,7 +617,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         // Disallow current Community ADD perms
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, c, Constants.ADD);
 
-        Collection result = collectionService.create(context, c);
+        collectionService.create(context, c);
         fail("Exception expected");
     }
 
@@ -672,7 +672,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         // Disallow current Community ADD perms
         doThrow(new AuthorizeException()).when(authorizeServiceSpy).authorizeAction(context, c, Constants.ADD);
 
-        Community result = communityService.createSubcommunity(context, c);
+        communityService.createSubcommunity(context, c);
         fail("Exception expected");
     }
 
@@ -872,8 +872,9 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         Collection grandchildCol = collectionService.create(context, grandchild);
         // Create two separate items
         WorkspaceItem wsItem = workspaceItemService.create(context, childCol, false);
-        wsItem = workspaceItemService.create(context, childCol, false);
         Item item = installItemService.installItem(context, wsItem);
+        wsItem = workspaceItemService.create(context, grandchildCol, false);
+        Item item2 = installItemService.installItem(context, wsItem);
 
         // Done creating the objects. Turn auth system back on
         context.restoreAuthSystemState();
@@ -885,6 +886,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
         UUID childColId = childCol.getID();
         UUID grandchildColId = grandchildCol.getID();
         UUID itemId = item.getID();
+        UUID item2Id = item2.getID();
 
         // Delete the parent of this entire hierarchy
         communityService.delete(context, parent);
@@ -902,6 +904,8 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
                    collectionService.find(context, grandchildColId), nullValue());
         assertThat("Item not deleted",
                    itemService.find(context, itemId), nullValue());
+        assertThat("Item not deleted",
+                   itemService.find(context, item2Id), nullValue());
     }
 
     /**
@@ -1030,7 +1034,7 @@ public class CommunityTest extends AbstractDSpaceObjectTest {
             assertThat("testGetParentObject 1", communityService.getParentObject(context, son), notNullValue());
             assertThat("testGetParentObject 2", (Community) communityService.getParentObject(context, son), equalTo(c));
         } catch (AuthorizeException ex) {
-            fail("Authorize exception catched");
+            throw new AssertionError("AuthorizeException occurred", ex);
         }
     }
 

dspace-api/src/test/java/org/dspace/content/LicenseUtilsTest.java

@@ -104,11 +104,11 @@ public class LicenseUtilsTest extends AbstractUnitTest {
     @Test
     public void testGetLicenseText_5args() throws SQLException, AuthorizeException, IOException {
         //parameters for the test
-        Locale locale = null;
+        Locale locale;
-        Collection collection = null;
+        Collection collection;
-        Item item = null;
+        Item item;
-        EPerson person = null;
+        EPerson person;
-        Map<String, Object> additionalInfo = null;
+        Map<String, Object> additionalInfo;
 
         // We don't test attribute 4 as this is the date, and the date often differs between when the test
         // is executed, and when the LicenceUtils code gets the current date/time which causes the test to fail
@@ -195,10 +195,10 @@ public class LicenseUtilsTest extends AbstractUnitTest {
     @Test
     public void testGetLicenseText_4args() throws SQLException, AuthorizeException, IOException {
         //parameters for the test
-        Locale locale = null;
+        Locale locale;
-        Collection collection = null;
+        Collection collection;
-        Item item = null;
+        Item item;
-        EPerson person = null;
+        EPerson person;
 
         String template = "Template license: %1$s %2$s %3$s %5$s %6$s";
         String templateResult = "Template license: first name last name testgetlicensetext_4args@email.com  ";

dspace-api/src/test/java/org/dspace/content/VersioningTest.java

@@ -25,7 +25,6 @@ import org.dspace.content.service.CommunityService;
 import org.dspace.content.service.InstallItemService;
 import org.dspace.content.service.ItemService;
 import org.dspace.content.service.WorkspaceItemService;
-import org.dspace.core.ConfigurationManager;
 import org.dspace.handle.factory.HandleServiceFactory;
 import org.dspace.handle.service.HandleService;
 import org.dspace.versioning.Version;
@@ -49,7 +48,6 @@ public class VersioningTest extends AbstractUnitTest {
 
     private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(VersioningTest.class);
 
-    private String originalHandle;
     private Item originalItem;
     private Item versionedItem;
     private String summary = "Unit test version";
@@ -65,7 +63,7 @@ public class VersioningTest extends AbstractUnitTest {
 
     //A regex that can be used to see if a handle contains the format of handle created by the org.dspace.identifier
     // .VersionedHandleIdentifierProvider*
-    protected String versionedHandleRegex = ConfigurationManager.getProperty("handle.prefix") + "\\/[0-9]*\\.[0-9]";
+    //protected String versionedHandleRegex = ConfigurationManager.getProperty("handle.prefix") + "\\/[0-9]*\\.[0-9]";
 
     /**
      * This method will be run before every test as per @Before. It will
@@ -86,7 +84,6 @@ public class VersioningTest extends AbstractUnitTest {
             WorkspaceItem is = workspaceItemService.create(context, col, false);
 
             originalItem = installItemService.installItem(context, is);
-            originalHandle = originalItem.getHandle();
 
             Version version = versionService.createNewVersion(context, originalItem, summary);
             WorkspaceItem wsi = workspaceItemService.findByItem(context, version.getItem());

dspace-api/src/test/java/org/dspace/xmlworkflow/RoleTest.java

@@ -31,49 +31,48 @@ public class RoleTest extends AbstractUnitTest {
     @Test
     public void defaultWorkflow_RoleReviewer() {
         Role role = defaultWorkflow.getRoles().get("Reviewer");
-        assertEquals(role.getDescription(),
+        assertEquals("The people responsible for this step are able to edit the metadata of incoming submissions, " +
-                "The people responsible for this step are able to edit the metadata of incoming submissions, " +
+                        "and then accept or reject them.", role.getDescription());
-                        "and then accept or reject them.");
+        assertEquals("Reviewer", role.getName());
-        assertEquals(role.getName(), "Reviewer");
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
     }
 
     @Test
     public void defaultWorkflow_RoleEditor() {
         Role role = defaultWorkflow.getRoles().get("Editor");
-        assertEquals(role.getDescription(), "The people responsible for this step are able to edit the " +
+        assertEquals("The people responsible for this step are able to edit the " +
-                "metadata of incoming submissions, and then accept or reject them.");
+                "metadata of incoming submissions, and then accept or reject them.", role.getDescription());
-        assertEquals(role.getName(), "Editor");
+        assertEquals("Editor", role.getName());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
     }
 
     @Test
     public void defaultWorkflow_RoleFinalEditor() {
         Role role = defaultWorkflow.getRoles().get("Final Editor");
-        assertEquals(role.getDescription(), "The people responsible for this step are able to edit the " +
+        assertEquals("The people responsible for this step are able to edit the " +
-                "metadata of incoming submissions, but will not be able to reject them.");
+                "metadata of incoming submissions, but will not be able to reject them.", role.getDescription());
-        assertEquals(role.getName(), "Final Editor");
+        assertEquals("Final Editor", role.getName());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
     }
 
     @Test
     public void selectSingleReviewer_RoleReviewManagers() {
         Role role = selectSingleReviewer.getRoles().get("ReviewManagers");
-        assertEquals(role.getName(), "ReviewManagers");
+        assertEquals("ReviewManagers", role.getName());
-        assertEquals(role.getScope(), Role.Scope.REPOSITORY);
+        assertEquals(Role.Scope.REPOSITORY, role.getScope());
     }
 
     @Test
     public void selectSingleReviewer_RoleReviewer() {
         Role role = selectSingleReviewer.getRoles().get("Reviewer");
-        assertEquals(role.getName(), "Reviewer");
+        assertEquals("Reviewer", role.getName());
-        assertEquals(role.getScope(), Role.Scope.ITEM);
+        assertEquals(Role.Scope.ITEM, role.getScope());
     }
 
     @Test
     public void scoreReview_RoleScoreReviewers() {
         Role role = scoreReview.getRoles().get("ScoreReviewers");
-        assertEquals(role.getName(), "ScoreReviewers");
+        assertEquals("ScoreReviewers", role.getName());
-        assertEquals(role.getScope(), Role.Scope.COLLECTION);
+        assertEquals(Role.Scope.COLLECTION, role.getScope());
     }
 }

dspace-api/src/test/java/org/dspace/xmlworkflow/state/WorkflowTest.java

@@ -7,7 +7,8 @@
  */
 package org.dspace.xmlworkflow.state;
 
-import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.List;
 
@@ -31,30 +32,30 @@ public class WorkflowTest extends AbstractUnitTest {
 
     @Test
     public void defaultWorkflow() {
-        assertEquals(defaultWorkflow.getFirstStep().getId(), "reviewstep");
+        assertEquals("reviewstep", defaultWorkflow.getFirstStep().getId());
         List<Step> steps = defaultWorkflow.getSteps();
-        assertEquals(steps.size(), 3);
+        assertEquals(3, steps.size());
-        assert (this.containsStepNamed(steps, "reviewstep"));
+        assertTrue(this.containsStepNamed(steps, "reviewstep"));
-        assert (this.containsStepNamed(steps, "editstep"));
+        assertTrue(this.containsStepNamed(steps, "editstep"));
-        assert (this.containsStepNamed(steps, "finaleditstep"));
+        assertTrue(this.containsStepNamed(steps, "finaleditstep"));
     }
 
     @Test
     public void selectSingleReviewer() {
-        assertEquals(selectSingleReviewer.getFirstStep().getId(), "selectReviewerStep");
+        assertEquals("selectReviewerStep", selectSingleReviewer.getFirstStep().getId());
         List<Step> steps = selectSingleReviewer.getSteps();
-        assertEquals(steps.size(), 2);
+        assertEquals(2, steps.size());
-        assert (this.containsStepNamed(steps, "selectReviewerStep"));
+        assertTrue(this.containsStepNamed(steps, "selectReviewerStep"));
-        assert (this.containsStepNamed(steps, "singleUserReviewStep"));
+        assertTrue(this.containsStepNamed(steps, "singleUserReviewStep"));
     }
 
     @Test
     public void scoreReview() {
-        assertEquals(scoreReview.getFirstStep().getId(), "scoreReviewStep");
+        assertEquals("scoreReviewStep", scoreReview.getFirstStep().getId());
         List<Step> steps = scoreReview.getSteps();
-        assertEquals(steps.size(), 2);
+        assertEquals(2, steps.size());
-        assert (this.containsStepNamed(steps, "scoreReviewStep"));
+        assertTrue(this.containsStepNamed(steps, "scoreReviewStep"));
-        assert (this.containsStepNamed(steps, "evaluationStep"));
+        assertTrue(this.containsStepNamed(steps, "evaluationStep"));
     }
 
     private boolean containsStepNamed(List<Step> steps, String stepName) {