This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/3.1 by this push:
new d0ee570819 Remove niceMocks (#5255)
d0ee570819 is described below
commit d0ee570819e05c1deaa47ea213b660cc18076dce
Author: Christopher Tubbs <[email protected]>
AuthorDate: Tue Jan 14 14:06:20 2025 -0500
Remove niceMocks (#5255)
* Remove the use of mock objects created using EasyMock.niceMock to
improve the rigor of our test coverage a little, and the reliability
of those affected tests
Trivial changes in related non-test code:
* make use of return value from Objects.requireNonNull
* avoid passing redundant AccumuloConfiguration object when
ServerContext suffices
---
.../data/constraints/VisibilityConstraintTest.java | 25 +++++------
.../file/streams/RateLimitedInputStreamTest.java | 2 +-
.../file/streams/RateLimitedOutputStreamTest.java | 2 +-
.../accumulo/tserver/ActiveAssignmentRunnable.java | 9 ++--
.../tserver/TabletServerResourceManager.java | 10 ++---
.../accumulo/tserver/AssignmentWatcherTest.java | 48 +++++++++-------------
.../shell/format/DeleterFormatterTest.java | 14 ++++---
7 files changed, 51 insertions(+), 59 deletions(-)
diff --git
a/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
b/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
index 2f31877701..dd8583b7ea 100644
---
a/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
@@ -20,9 +20,9 @@ package org.apache.accumulo.core.data.constraints;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.createNiceMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -32,16 +32,16 @@ import java.util.List;
import org.apache.accumulo.core.data.ArrayByteSequence;
import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.constraints.Constraint.Environment;
-import org.apache.accumulo.core.security.AuthorizationContainer;
import org.apache.accumulo.core.security.ColumnVisibility;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
public class VisibilityConstraintTest {
- VisibilityConstraint vc;
- Environment env;
- Mutation mutation;
+ private VisibilityConstraint vc;
+ private Environment env;
+ private Mutation mutation;
static final ColumnVisibility good = new ColumnVisibility("good|bad");
static final ColumnVisibility bad = new ColumnVisibility("good&bad");
@@ -57,17 +57,18 @@ public class VisibilityConstraintTest {
vc = new VisibilityConstraint();
mutation = new Mutation("r");
- ArrayByteSequence bs = new ArrayByteSequence("good".getBytes(UTF_8));
-
- AuthorizationContainer ac = createNiceMock(AuthorizationContainer.class);
- expect(ac.contains(bs)).andReturn(true);
- replay(ac);
-
env = createMock(Environment.class);
- expect(env.getAuthorizationsContainer()).andReturn(ac);
+ expect(env.getAuthorizationsContainer())
+ .andReturn(new
ArrayByteSequence("good".getBytes(UTF_8))::equals).anyTimes();
+
replay(env);
}
+ @AfterEach
+ public void tearDown() {
+ verify(env);
+ }
+
@Test
public void testNoVisibility() {
mutation.put(D, D, D);
diff --git
a/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedInputStreamTest.java
b/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedInputStreamTest.java
index 8eb3a5ef67..f56d1733b7 100644
---
a/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedInputStreamTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedInputStreamTest.java
@@ -36,7 +36,7 @@ public class RateLimitedInputStreamTest {
// Create variables for tracking behaviors of mock object
AtomicLong rateLimiterPermitsAcquired = new AtomicLong();
// Construct mock object
- RateLimiter rateLimiter = EasyMock.niceMock(RateLimiter.class);
+ RateLimiter rateLimiter = EasyMock.createMock(RateLimiter.class);
// Stub Mock Method
rateLimiter.acquire(EasyMock.anyLong());
EasyMock.expectLastCall()
diff --git
a/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedOutputStreamTest.java
b/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedOutputStreamTest.java
index 8df1a3104e..5f1b25da11 100644
---
a/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedOutputStreamTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/file/streams/RateLimitedOutputStreamTest.java
@@ -38,7 +38,7 @@ public class RateLimitedOutputStreamTest {
// Create variables for tracking behaviors of mock object
AtomicLong rateLimiterPermitsAcquired = new AtomicLong();
// Construct mock object
- RateLimiter rateLimiter = EasyMock.niceMock(RateLimiter.class);
+ RateLimiter rateLimiter = EasyMock.createMock(RateLimiter.class);
// Stub Mock Method
rateLimiter.acquire(EasyMock.anyLong());
EasyMock.expectLastCall()
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java
index 3ce9cc1689..b5cedfbb4e 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java
@@ -39,12 +39,9 @@ public class ActiveAssignmentRunnable implements Runnable {
public
ActiveAssignmentRunnable(ConcurrentHashMap<KeyExtent,RunnableStartedAt>
activeAssignments,
KeyExtent extent, Runnable delegate) {
- requireNonNull(activeAssignments);
- requireNonNull(extent);
- requireNonNull(delegate);
- this.activeAssignments = activeAssignments;
- this.extent = extent;
- this.delegate = delegate;
+ this.activeAssignments = requireNonNull(activeAssignments);
+ this.extent = requireNonNull(extent);
+ this.delegate = requireNonNull(delegate);
}
@Override
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
index d6afc3e022..1228ac1fbd 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
@@ -398,8 +398,8 @@ public class TabletServerResourceManager {
// We can use the same map for both metadata and normal assignments since
the keyspace (extent)
// is guaranteed to be unique. Schedule the task once, the task will
reschedule itself.
-
ThreadPools.watchCriticalScheduledTask(context.getScheduledExecutor().schedule(
- new AssignmentWatcher(acuConf, context, activeAssignments), 5000,
TimeUnit.MILLISECONDS));
+ ThreadPools.watchCriticalScheduledTask(context.getScheduledExecutor()
+ .schedule(new AssignmentWatcher(context, activeAssignments), 5000,
TimeUnit.MILLISECONDS));
}
public int getOpenFiles() {
@@ -417,16 +417,14 @@ public class TabletServerResourceManager {
private static long longAssignments = 0;
private final Map<KeyExtent,RunnableStartedAt> activeAssignments;
- private final AccumuloConfiguration conf;
private final ServerContext context;
public static long getLongAssignments() {
return longAssignments;
}
- public AssignmentWatcher(AccumuloConfiguration conf, ServerContext context,
+ public AssignmentWatcher(ServerContext context,
Map<KeyExtent,RunnableStartedAt> activeAssignments) {
- this.conf = conf;
this.context = context;
this.activeAssignments = activeAssignments;
}
@@ -434,7 +432,7 @@ public class TabletServerResourceManager {
@Override
public void run() {
final long millisBeforeWarning =
-
this.conf.getTimeInMillis(Property.TSERV_ASSIGNMENT_DURATION_WARNING);
+
context.getConfiguration().getTimeInMillis(Property.TSERV_ASSIGNMENT_DURATION_WARNING);
try {
long now = System.currentTimeMillis();
KeyExtent extent;
diff --git
a/server/tserver/src/test/java/org/apache/accumulo/tserver/AssignmentWatcherTest.java
b/server/tserver/src/test/java/org/apache/accumulo/tserver/AssignmentWatcherTest.java
index 86899b5b66..4a90cca43f 100644
---
a/server/tserver/src/test/java/org/apache/accumulo/tserver/AssignmentWatcherTest.java
+++
b/server/tserver/src/test/java/org/apache/accumulo/tserver/AssignmentWatcherTest.java
@@ -18,51 +18,43 @@
*/
package org.apache.accumulo.tserver;
-import java.util.HashMap;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+
import java.util.Map;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledThreadPoolExecutor;
-import org.apache.accumulo.core.conf.AccumuloConfiguration;
+import org.apache.accumulo.core.conf.ConfigurationCopy;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.server.ServerContext;
import
org.apache.accumulo.tserver.TabletServerResourceManager.AssignmentWatcher;
-import org.easymock.EasyMock;
-import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
public class AssignmentWatcherTest {
- private Map<KeyExtent,RunnableStartedAt> assignments;
- private ServerContext context;
- private AccumuloConfiguration conf;
- private AssignmentWatcher watcher;
-
- @BeforeEach
- public void setup() {
- assignments = new HashMap<>();
- context = EasyMock.createMock(ServerContext.class);
- conf = EasyMock.createNiceMock(AccumuloConfiguration.class);
- watcher = new AssignmentWatcher(conf, context, assignments);
- }
-
@Test
public void testAssignmentWarning() {
- ActiveAssignmentRunnable task =
EasyMock.createMock(ActiveAssignmentRunnable.class);
- RunnableStartedAt run = new RunnableStartedAt(task,
System.currentTimeMillis());
- EasyMock.expect(context.getConfiguration()).andReturn(conf).anyTimes();
-
EasyMock.expect(conf.getCount(EasyMock.isA(Property.class))).andReturn(1).anyTimes();
-
EasyMock.expect(conf.getTimeInMillis(EasyMock.isA(Property.class))).andReturn(0L).anyTimes();
- EasyMock.expect(context.getScheduledExecutor())
- .andReturn((ScheduledThreadPoolExecutor)
Executors.newScheduledThreadPool(1)).anyTimes();
- assignments.put(new KeyExtent(TableId.of("1"), null, null), run);
+ var e = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(1);
+ var c = new
ConfigurationCopy(Map.of(Property.TSERV_ASSIGNMENT_DURATION_WARNING.getKey(),
"0"));
+ ServerContext context = createMock(ServerContext.class);
+ expect(context.getScheduledExecutor()).andReturn(e);
+ expect(context.getConfiguration()).andReturn(c);
+
+ ActiveAssignmentRunnable task = createMock(ActiveAssignmentRunnable.class);
+ expect(task.getException()).andReturn(new Exception("Assignment warning
happened"));
+
+ var assignments = Map.of(new KeyExtent(TableId.of("1"), null, null),
+ new RunnableStartedAt(task, System.currentTimeMillis()));
+ var watcher = new AssignmentWatcher(context, assignments);
- EasyMock.expect(task.getException()).andReturn(new Exception("Assignment
warning happened"));
- EasyMock.replay(context, task);
+ replay(context, task);
watcher.run();
- EasyMock.verify(context, task);
+ verify(context, task);
}
}
diff --git
a/shell/src/test/java/org/apache/accumulo/shell/format/DeleterFormatterTest.java
b/shell/src/test/java/org/apache/accumulo/shell/format/DeleterFormatterTest.java
index bb6825ed49..86b8374004 100644
---
a/shell/src/test/java/org/apache/accumulo/shell/format/DeleterFormatterTest.java
+++
b/shell/src/test/java/org/apache/accumulo/shell/format/DeleterFormatterTest.java
@@ -21,7 +21,6 @@ package org.apache.accumulo.shell.format;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.createNiceMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
@@ -85,12 +84,17 @@ public class DeleterFormatterTest {
}
@BeforeEach
- public void setUp() throws IOException {
+ public void setUp() throws Exception {
input = new SettableInputStream();
baos = new ByteArrayOutputStream();
- writer = createNiceMock(BatchWriter.class);
- shellState = createNiceMock(Shell.class);
+ writer = createMock(BatchWriter.class);
+ writer.addMutation(anyObject());
+ expectLastCall().anyTimes();
+ writer.close();
+ expectLastCall().anyTimes();
+
+ shellState = createMock(Shell.class);
terminal = new DumbTerminal(input, baos);
terminal.setSize(new Size(80, 24));
@@ -197,7 +201,7 @@ public class DeleterFormatterTest {
@Test
public void testMutationException() throws MutationsRejectedException {
MutationsRejectedException mre =
createMock(MutationsRejectedException.class);
- BatchWriter exceptionWriter = createNiceMock(BatchWriter.class);
+ BatchWriter exceptionWriter = createMock(BatchWriter.class);
exceptionWriter.close();
expectLastCall().andThrow(mre);
exceptionWriter.addMutation(anyObject(Mutation.class));
