keith-turner commented on code in PR #3817: URL: https://github.com/apache/accumulo/pull/3817#discussion_r1349219961
########## src/build/ci/find-unapproved-junit.sh: ########## @@ -38,8 +36,10 @@ function findalljunitproblems() { fi # find any new classes using something other than the jupiter API, except those allowed grep "$opts" --include='*.java' 'org[.]junit[.](?!jupiter)' | grep -Pv "^(${ALLOWED_PIPE_SEP//./[.]})\$" - # find any uses of the jupiter API in the allowed vintage classes - grep "$opts" 'org[.]junit[.]jupiter' "${ALLOWED[@]}" + if ((${#ALLOWED[@]} != 0)); then Review Comment: Could be a follow on issue, wondering if the ALLOWED set should just be removed from the script since its empty. Would simplify the script. ########## server/compaction-coordinator/src/test/java/org/apache/accumulo/coordinator/CompactionCoordinatorTest.java: ########## @@ -1,657 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.accumulo.coordinator; - -import static org.easymock.EasyMock.anyObject; -import static org.easymock.EasyMock.expect; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import java.net.UnknownHostException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; -import java.util.TreeMap; -import java.util.TreeSet; -import java.util.UUID; -import java.util.concurrent.ScheduledThreadPoolExecutor; - -import org.apache.accumulo.core.cli.ConfigOpts; -import org.apache.accumulo.core.clientImpl.thrift.TInfo; -import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException; -import org.apache.accumulo.core.compaction.thrift.TExternalCompaction; -import org.apache.accumulo.core.conf.DefaultConfiguration; -import org.apache.accumulo.core.dataImpl.thrift.TKeyExtent; -import org.apache.accumulo.core.metadata.TServerInstance; -import org.apache.accumulo.core.metadata.schema.ExternalCompactionId; -import org.apache.accumulo.core.rpc.ThriftUtil; -import org.apache.accumulo.core.securityImpl.thrift.TCredentials; -import org.apache.accumulo.core.tabletserver.thrift.TCompactionQueueSummary; -import org.apache.accumulo.core.tabletserver.thrift.TCompactionStats; -import org.apache.accumulo.core.tabletserver.thrift.TExternalCompactionJob; -import org.apache.accumulo.core.tabletserver.thrift.TabletServerClientService; -import org.apache.accumulo.core.tabletserver.thrift.TabletServerClientService.Client; -import org.apache.accumulo.core.trace.TraceUtil; -import org.apache.accumulo.core.util.compaction.ExternalCompactionUtil; -import org.apache.accumulo.core.util.compaction.RunningCompaction; -import org.apache.accumulo.server.AbstractServer; -import org.apache.accumulo.server.ServerContext; -import org.apache.accumulo.server.manager.LiveTServerSet; -import org.apache.accumulo.server.rpc.ServerAddress; -import org.apache.accumulo.server.security.AuditedSecurityOperation; -import org.apache.thrift.transport.TTransportException; -import org.apache.zookeeper.KeeperException; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.core.classloader.annotations.SuppressStaticInitializationFor; -import org.powermock.modules.junit4.PowerMockRunner; - -import com.google.common.collect.Sets; -import com.google.common.net.HostAndPort; - -@RunWith(PowerMockRunner.class) -@PrepareForTest({CompactionCoordinator.class, DeadCompactionDetector.class, ThriftUtil.class, - ExternalCompactionUtil.class}) -@SuppressStaticInitializationFor({"org.apache.log4j.LogManager"}) -@PowerMockIgnore({"org.slf4j.*", "org.apache.logging.*", "org.apache.log4j.*", - "org.apache.commons.logging.*", "org.xml.*", "javax.xml.*", "org.w3c.dom.*", - "com.sun.org.apache.xerces.*"}) -public class CompactionCoordinatorTest { - - public class TestCoordinator extends CompactionCoordinator { - - private final ServerContext context; - private final ServerAddress client; - private final Client tabletServerClient; - - private Set<ExternalCompactionId> metadataCompactionIds = null; - - protected TestCoordinator(CompactionFinalizer finalizer, LiveTServerSet tservers, - ServerAddress client, Client tabletServerClient, ServerContext context, - AuditedSecurityOperation security) { - super(new ConfigOpts(), new String[] {}, context.getConfiguration()); - this.compactionFinalizer = finalizer; - this.tserverSet = tservers; - this.client = client; - this.tabletServerClient = tabletServerClient; - this.context = context; - this.security = security; - } - - @Override - protected void startDeadCompactionDetector() {} - - @Override - protected long getTServerCheckInterval() { - this.shutdown = true; - return 0L; - } - - @Override - protected void startCompactionCleaner(ScheduledThreadPoolExecutor schedExecutor) {} - - @Override - protected CompactionFinalizer createCompactionFinalizer(ScheduledThreadPoolExecutor stpe) { - return null; - } - - @Override - protected LiveTServerSet createLiveTServerSet() { - return null; - } - - @Override - protected void setupSecurity() {} - - @Override - protected void printStartupMsg() {} - - @Override - public ServerContext getContext() { - return this.context; - } - - @Override - protected void getCoordinatorLock(HostAndPort clientAddress) - throws KeeperException, InterruptedException {} - - @Override - protected ServerAddress startCoordinatorClientService() throws UnknownHostException { - return client; - } - - @Override - protected Client getTabletServerConnection(TServerInstance tserver) throws TTransportException { - return tabletServerClient; - } - - @Override - public void compactionCompleted(TInfo tinfo, TCredentials credentials, - String externalCompactionId, TKeyExtent textent, TCompactionStats stats) - throws ThriftSecurityException {} - - @Override - public void compactionFailed(TInfo tinfo, TCredentials credentials, String externalCompactionId, - TKeyExtent extent) throws ThriftSecurityException {} - - void setMetadataCompactionIds(Set<ExternalCompactionId> mci) { - metadataCompactionIds = mci; - } - - @Override - protected Set<ExternalCompactionId> readExternalCompactionIds() { - if (metadataCompactionIds == null) { - return RUNNING_CACHE.keySet(); - } else { - return metadataCompactionIds; - } - } - - public Map<String,TreeMap<Short,TreeSet<TServerInstance>>> getQueues() { - return CompactionCoordinator.QUEUE_SUMMARIES.QUEUES; - } - - public Map<TServerInstance,Set<QueueAndPriority>> getIndex() { - return CompactionCoordinator.QUEUE_SUMMARIES.INDEX; - } - - public Map<ExternalCompactionId,RunningCompaction> getRunning() { - return RUNNING_CACHE; - } - - public void resetInternals() { - getQueues().clear(); - getIndex().clear(); - getRunning().clear(); - metadataCompactionIds = null; - } - - } - - @Test - public void testCoordinatorColdStartNoCompactions() throws Exception { - PowerMock.resetAll(); - PowerMock.suppress(PowerMock.constructor(AbstractServer.class)); - PowerMock.suppress(PowerMock.methods(ThriftUtil.class, "returnClient")); - PowerMock.suppress(PowerMock.methods(DeadCompactionDetector.class, "detectDeadCompactions", - "detectDanglingFinalStateMarkers")); - - ServerContext context = PowerMock.createNiceMock(ServerContext.class); - expect(context.getConfiguration()).andReturn(DefaultConfiguration.getInstance()).anyTimes(); - - PowerMock.mockStatic(ExternalCompactionUtil.class); - List<RunningCompaction> runningCompactions = new ArrayList<>(); - expect(ExternalCompactionUtil.getCompactionsRunningOnCompactors(context)) - .andReturn(runningCompactions); - - CompactionFinalizer finalizer = PowerMock.createNiceMock(CompactionFinalizer.class); - LiveTServerSet tservers = PowerMock.createNiceMock(LiveTServerSet.class); - expect(tservers.getCurrentServers()).andReturn(Collections.emptySet()).anyTimes(); - - ServerAddress client = PowerMock.createNiceMock(ServerAddress.class); - HostAndPort address = HostAndPort.fromString("localhost:10240"); - expect(client.getAddress()).andReturn(address).anyTimes(); - - TServerInstance tsi = PowerMock.createNiceMock(TServerInstance.class); - expect(tsi.getHostPort()).andReturn("localhost:9997").anyTimes(); - - TabletServerClientService.Client tsc = - PowerMock.createNiceMock(TabletServerClientService.Client.class); - expect(tsc.getCompactionQueueInfo(anyObject(), anyObject())).andReturn(Collections.emptyList()) - .anyTimes(); - - AuditedSecurityOperation security = PowerMock.createNiceMock(AuditedSecurityOperation.class); - - PowerMock.replayAll(); - - var coordinator = new TestCoordinator(finalizer, tservers, client, tsc, context, security); - coordinator.resetInternals(); - assertEquals(0, coordinator.getQueues().size()); - assertEquals(0, coordinator.getIndex().size()); - assertEquals(0, coordinator.getRunning().size()); - coordinator.run(); - assertEquals(0, coordinator.getQueues().size()); - assertEquals(0, coordinator.getIndex().size()); - assertEquals(0, coordinator.getRunning().size()); - - PowerMock.verifyAll(); - coordinator.resetInternals(); - coordinator.close(); - } - - @Test - public void testCoordinatorColdStart() throws Exception { - PowerMock.resetAll(); - PowerMock.suppress(PowerMock.constructor(AbstractServer.class)); - PowerMock.suppress(PowerMock.methods(ThriftUtil.class, "returnClient")); - PowerMock.suppress(PowerMock.methods(DeadCompactionDetector.class, "detectDeadCompactions", - "detectDanglingFinalStateMarkers")); - - ServerContext context = PowerMock.createNiceMock(ServerContext.class); - expect(context.getConfiguration()).andReturn(DefaultConfiguration.getInstance()).anyTimes(); - - TCredentials creds = PowerMock.createNiceMock(TCredentials.class); - expect(context.rpcCreds()).andReturn(creds); - - PowerMock.mockStatic(ExternalCompactionUtil.class); - List<RunningCompaction> runningCompactions = new ArrayList<>(); - expect(ExternalCompactionUtil.getCompactionsRunningOnCompactors(context)) - .andReturn(runningCompactions); - - CompactionFinalizer finalizer = PowerMock.createNiceMock(CompactionFinalizer.class); - LiveTServerSet tservers = PowerMock.createNiceMock(LiveTServerSet.class); - TServerInstance instance = PowerMock.createNiceMock(TServerInstance.class); - expect(tservers.getCurrentServers()).andReturn(Collections.singleton(instance)).once(); - - ServerAddress client = PowerMock.createNiceMock(ServerAddress.class); - HostAndPort address = HostAndPort.fromString("localhost:10240"); - expect(client.getAddress()).andReturn(address).anyTimes(); - - TServerInstance tsi = PowerMock.createNiceMock(TServerInstance.class); - expect(tsi.getHostPort()).andReturn("localhost:9997").anyTimes(); - - TabletServerClientService.Client tsc = - PowerMock.createNiceMock(TabletServerClientService.Client.class); - TCompactionQueueSummary queueSummary = PowerMock.createNiceMock(TCompactionQueueSummary.class); - expect(tsc.getCompactionQueueInfo(anyObject(), anyObject())) - .andReturn(Collections.singletonList(queueSummary)).anyTimes(); - expect(queueSummary.getQueue()).andReturn("R2DQ").anyTimes(); - expect(queueSummary.getPriority()).andReturn((short) 1).anyTimes(); - - AuditedSecurityOperation security = PowerMock.createNiceMock(AuditedSecurityOperation.class); - - PowerMock.replayAll(); - - var coordinator = new TestCoordinator(finalizer, tservers, client, tsc, context, security); - coordinator.resetInternals(); - assertEquals(0, coordinator.getQueues().size()); - assertEquals(0, coordinator.getIndex().size()); - assertEquals(0, coordinator.getRunning().size()); - coordinator.run(); - assertEquals(1, coordinator.getQueues().size()); - QueueAndPriority qp = QueueAndPriority.get("R2DQ".intern(), (short) 1); Review Comment: we are losing `R2DQ` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org