dlmarion commented on code in PR #5811: URL: https://github.com/apache/accumulo/pull/5811#discussion_r2304022781
########## test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java: ########## @@ -0,0 +1,314 @@ +/* + * 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.test.functional; + +import static org.apache.accumulo.harness.AccumuloITBase.MINI_CLUSTER_ONLY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Constructor; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.Function; +import java.util.stream.Stream; + +import org.apache.accumulo.compactor.Compactor; +import org.apache.accumulo.core.cli.ConfigOpts; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.conf.SiteConfiguration; +import org.apache.accumulo.gc.SimpleGarbageCollector; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.manager.Manager; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl.ProcessInfo; +import org.apache.accumulo.server.AbstractServer; +import org.apache.accumulo.server.ServerContext; +import org.apache.accumulo.test.functional.ExitCodesIT.ProcessProxy.TerminalBehavior; +import org.apache.accumulo.test.util.Wait; +import org.apache.accumulo.tserver.ScanServer; +import org.apache.accumulo.tserver.TabletServer; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.slf4j.LoggerFactory; + +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; +import net.bytebuddy.implementation.ExceptionMethod; +import net.bytebuddy.implementation.Implementation; +import net.bytebuddy.implementation.MethodCall; +import net.bytebuddy.matcher.ElementMatchers; + +@Tag(MINI_CLUSTER_ONLY) +public class ExitCodesIT extends SharedMiniClusterBase { + + public static class ServerContextFunction implements Function<SiteConfiguration,ServerContext> { + + @Override + public ServerContext apply(SiteConfiguration site) { + return new ServerContext(site); + } + + } + + // Dynamic Proxy class for a server instance that will be invoked via + // Accumulo's Main class using MiniAccumuloClusterImpl._exec(). This + // ProcessProxy class will determine which class to instantiate + // and what the terminal behavior should be using two system + // properties. A subclass of the desired class is dynamically created + // and started. + public static class ProcessProxy { + + public static enum TerminalBehavior { + SHUTDOWN, EXCEPTION, ERROR + }; + + public static final String PROXY_CLASS = "proxy.server"; + public static final String PROXY_METHOD_BEHAVIOR = "proxy.method.behavior"; + + public static void main(String[] args) throws Exception { + + final String proxyServer = System.getProperty(PROXY_CLASS); + Objects.requireNonNull(proxyServer, PROXY_CLASS + " must exist as an env var"); + final String methodBehavior = System.getProperty(PROXY_METHOD_BEHAVIOR); + Objects.requireNonNull(methodBehavior, methodBehavior + " must exist as an env var"); + + final ServerType st = ServerType.valueOf(proxyServer); + final TerminalBehavior behavior = TerminalBehavior.valueOf(methodBehavior); + + // Determine the constructor arguments and parameters for each server class. + // Find a method with no-args that does not return anything that is + // called during the servers run method that we can intercept to signal + // shutdown, exception, or error. + final Class<? extends AbstractServer> serverClass; + final String methodName; + final Class<?>[] ctorParams; + final Object[] ctorArgs; + switch (st) { + case COMPACTOR: + List<String> compactorArgs = new ArrayList<>(); + compactorArgs.add("-o"); + compactorArgs.add(Property.COMPACTOR_GROUP_NAME.getKey() + "=TEST"); + serverClass = Compactor.class; + methodName = "updateIdleStatus"; Review Comment: > What happens w/ the test if this method does not exists? Does the test fail? Seems like maybe it would fail because the SHUTDOWN case would expect an exit code of zero but would see some other exit code. But not sure. I can make it fail explicity by adding the following to ProcessProxy.main: ``` diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java index 4f8f948d26..c17a30ab6e 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ExitCodesIT.java @@ -155,6 +155,15 @@ public class ExitCodesIT extends SharedMiniClusterBase { throw new UnsupportedOperationException(st + " is not currently supported"); } + // Check that methodName exists on serverClass + try { + @SuppressWarnings("unused") + var ignored = serverClass.getDeclaredMethod(methodName); + } catch (NoSuchMethodException nsme) { + nsme.printStackTrace(); + System.exit(42); + } + ``` > Wondering if instead of this run time overriding if it would be possible to do it at compile time instead w/ something like the following. Then if someone changes a method name they will get a compile time error. I think I could make something that is functionally equivalent without using ByteBuddy, but it would be more code. I started with ByteBuddy initially because I wasn't sure of the design when I started, but I knew that I basically needed a proxy for each server class. -- 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
