+1 to Galen. I was thinking about the GeodeAwaitility vs. Awaitility rule, but that one only needed the rule because we do still depend on Awaitility.
On Wed, Dec 5, 2018 at 1:49 PM Alexander Murmann <amurm...@pivotal.io> wrote: > +1 to Galen's point. We already follow a PR process and if a committer > bypasses that to sneak PowerMock back in, it seems like we have much larger > problems. > > On Wed, Dec 5, 2018 at 10:35 AM Galen O'Sullivan <gosulli...@pivotal.io> > wrote: > > > Can we just remove PowerMock from our dependencies and skip the rule? I'd > > like to hope we can control our dependencies reasonably well. > > > > On Tue, Dec 4, 2018 at 4:45 PM Ryan McMahon <mcmellaw...@apache.org> > > wrote: > > > > > +1 to a spotless rule. Unless anyone objects, I’ll look into doing > that > > > after PowerMock is eliminated. > > > > > > Ryan > > > > > > On Tue, Dec 4, 2018 at 3:50 PM Helena Bales <hba...@pivotal.io> wrote: > > > > > > > Once we have refactored tests currently using PowerMock, it might be > > > > prudent to introduce a spotless rule to prohibit the reintroduction > of > > > > PowerMock. > > > > > > > > On Tue, Dec 4, 2018 at 3:32 PM Ryan McMahon <mcmellaw...@apache.org> > > > > wrote: > > > > > > > > > I am interested in contributing to this effort. I removed > PowerMock > > > > usage > > > > > from one set of tests (GEODE-6052), and at that time I took a quick > > > > glance > > > > > at other usages. I’ll assign GEODE-6143 to myself and see about > > > removing > > > > > the remaining usages. > > > > > > > > > > Ryan > > > > > > > > > > On Tue, Dec 4, 2018 at 3:08 PM Kirk Lund <kl...@apache.org> wrote: > > > > > > > > > > > I filed GEODE-6143: PowerMock should not be used in Geode tests. > > > > > > > > > > > > We need everyone to stop using PowerMock in new tests. If anyone > > > sees a > > > > > PR > > > > > > attempting to use PowerMock please let the contributor know about > > > > > > GEODE-6143. The alternative is to refactor product code such that > > > > > > dependencies are passed into the constructor instead of reaching > > out > > > to > > > > > > singletons and to avoid using static methods or static final > fields > > > for > > > > > > anything would would make testing more difficult. > > > > > > > > > > > > On Tue, Dec 4, 2018 at 11:20 AM Dan Smith <dsm...@pivotal.io> > > wrote: > > > > > > > > > > > > > +1 to removing PowerMock. Any situation that needs PowerMock > > needs > > > > > > > refactoring more. > > > > > > > > > > > > > > -Dan > > > > > > > > > > > > > > On Tue, Dec 4, 2018 at 10:27 AM Kirk Lund <kl...@apache.org> > > > wrote: > > > > > > > > > > > > > > > Anyone have any ideas which unit test is using PowerMock and > is > > > > > > > injecting a > > > > > > > > mock ClassLoader? This keeps failing in my precheckin runs. I > > > think > > > > > we > > > > > > > need > > > > > > > > to a) remove all uses of PowerMock and b) forbid its use > going > > > > > forward. > > > > > > > > > > > > > > > > 2018-12-04 18:11:36,258 Distributed system shutdown hook > ERROR > > > > Could > > > > > > not > > > > > > > > reconfigure JMX java.lang.LinkageError: loader constraint > > > > violation: > > > > > > > loader > > > > > > > > (instance of org/powermock/core/classloader/MockClassLoader) > > > > > previously > > > > > > > > initiated loading for a different type with name > > > > > > > > "javax/management/MBeanServer" > > > > > > > > at java.lang.ClassLoader.defineClass1(Native Method) > > > > > > > > at java.lang.ClassLoader.defineClass(ClassLoader.java:763) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.powermock.core.classloader.MockClassLoader.loadUnmockedClass(MockClassLoader.java:262) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.powermock.core.classloader.MockClassLoader.loadModifiedClass(MockClassLoader.java:206) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.powermock.core.classloader.DeferSupportingClassLoader.loadClass1(DeferSupportingClassLoader.java:89) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.powermock.core.classloader.DeferSupportingClassLoader.loadClass(DeferSupportingClassLoader.java:79) > > > > > > > > at java.lang.ClassLoader.loadClass(ClassLoader.java:357) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.jmx.Server.unregisterAllMatching(Server.java:337) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.jmx.Server.unregisterLoggerContext(Server.java:261) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:165) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.jmx.Server.reregisterMBeansAfterReconfigure(Server.java:141) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:558) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:619) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:636) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:231) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:243) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:45) > > > > > > > > at > > > > > org.apache.logging.log4j.LogManager.getContext(LogManager.java:174) > > > > > > > > at > > > > org.apache.logging.log4j.LogManager.getLogger(LogManager.java:661) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.geode.internal.logging.LogService.getLogger(LogService.java:64) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.geode.internal.tcp.ConnectionTable.<clinit>(ConnectionTable.java:61) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.geode.distributed.DistributedSystem.setThreadsSocketPolicy(DistributedSystem.java:263) > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > org.apache.geode.distributed.internal.InternalDistributedSystem.lambda$static$0(InternalDistributedSystem.java:2317) > > > > > > > > at java.lang.Thread.run(Thread.java:748) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
