Re: PowerMock and mock ClassLoader

2018-12-05 Thread Galen O'Sullivan 
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  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  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 
> > 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  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  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 
> 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

Re: [DISCUSS] LGTM on pull requests

2018-12-05 Thread Bruce Schuchardt 

Maybe we need to poke infra about this

On 11/9/18 3:07 PM, Jacob Barrett wrote:

I opened a ticket with infra earlier this week to enable PR integration. There 
hasn’t been any movement.


On Nov 9, 2018, at 3:00 PM, Nabarun Nag  wrote:

As per running periodically , LGTM runs it every Monday.

As for who would fix it, LGTM mentions which commit caused the failure and who 
was the author of it. So i think its the author's responsibility to fix it.

Personally, LGTM list a table that shows how many alerts we caused by which author [ 
https://lgtm.com/projects/g/apache/geode/contributors:java 
 ]
I cleaning up whatever alerts I have introduced into Apache Geode.

Regards
Nabarun



On Nov 9, 2018, at 2:54 PM, Alexander Murmann  wrote:

I don't have strong opinions on this, but I am always suspect of CI jobs
that indicate quality that only run periodically. If the job discovers
something that needs improvement who is going to do the work and when?


On Fri, Nov 9, 2018 at 2:36 PM Kirk Lund  wrote:

Well, we could run it periodically such as weekly rather than as part of
the main pipeline or precheckin.

On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri 
wrote:


+1, although I do wonder about the overhead of making PRs increasing more
than it already feels like to me as a new contributor (as the person who
made the geospatial contribution). If this was a gradle task maybe like
spotless?

On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
wrote:


I'd like to see LGTM run on pull requests.  Otherwise I think we're
fighting a losing battle trying to improve the quality of our code. For
instance, we just had a nice contribution of geospatial functionality
that raised 5 alerts, but we only found out about it after the code was
merged to develop.

LGTM allows that kind of integration but you have to be the repo

"owner"

to set it up.


https://lgtm.com/projects/g/apache/geode/

Re: [DISCUSS] LGTM on pull requests

2018-12-05 Thread Jacob Barrett 
They are investigating security concerns around the integration.

> On Dec 5, 2018, at 11:05 AM, Bruce Schuchardt  wrote:
> 
> Maybe we need to poke infra about this
> 
>> On 11/9/18 3:07 PM, Jacob Barrett wrote:
>> I opened a ticket with infra earlier this week to enable PR integration. 
>> There hasn’t been any movement.
>> 
>>> On Nov 9, 2018, at 3:00 PM, Nabarun Nag  wrote:
>>> 
>>> As per running periodically , LGTM runs it every Monday.
>>> 
>>> As for who would fix it, LGTM mentions which commit caused the failure and 
>>> who was the author of it. So i think its the author's responsibility to fix 
>>> it.
>>> 
>>> Personally, LGTM list a table that shows how many alerts we caused by which 
>>> author [ https://lgtm.com/projects/g/apache/geode/contributors:java 
>>>  ]
>>> I cleaning up whatever alerts I have introduced into Apache Geode.
>>> 
>>> Regards
>>> Nabarun
>>> 
>>> 
 On Nov 9, 2018, at 2:54 PM, Alexander Murmann  wrote:
 
 I don't have strong opinions on this, but I am always suspect of CI jobs
 that indicate quality that only run periodically. If the job discovers
 something that needs improvement who is going to do the work and when?
 
> On Fri, Nov 9, 2018 at 2:36 PM Kirk Lund  wrote:
> 
> Well, we could run it periodically such as weekly rather than as part of
> the main pipeline or precheckin.
> 
> On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri 
> wrote:
> 
>> +1, although I do wonder about the overhead of making PRs increasing more
>> than it already feels like to me as a new contributor (as the person who
>> made the geospatial contribution). If this was a gradle task maybe like
>> spotless?
>> 
>> On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
>> wrote:
>> 
>>> I'd like to see LGTM run on pull requests.  Otherwise I think we're
>>> fighting a losing battle trying to improve the quality of our code. For
>>> instance, we just had a nice contribution of geospatial functionality
>>> that raised 5 alerts, but we only found out about it after the code was
>>> merged to develop.
>>> 
>>> LGTM allows that kind of integration but you have to be the repo
> "owner"
>>> to set it up.
>>> 
>>> 
>>> https://lgtm.com/projects/g/apache/geode/
>>> 
>>> 
>>>

Re: [DISCUSS] LGTM on pull requests

2018-12-05 Thread Jacob Barrett 
https://issues.apache.org/jira/browse/INFRA-17226

> On Dec 5, 2018, at 11:26 AM, Jacob Barrett  wrote:
> 
> They are investigating security concerns around the integration.
> 
>> On Dec 5, 2018, at 11:05 AM, Bruce Schuchardt  wrote:
>> 
>> Maybe we need to poke infra about this
>> 
>>> On 11/9/18 3:07 PM, Jacob Barrett wrote:
>>> I opened a ticket with infra earlier this week to enable PR integration. 
>>> There hasn’t been any movement.
>>> 
 On Nov 9, 2018, at 3:00 PM, Nabarun Nag  wrote:
 
 As per running periodically , LGTM runs it every Monday.
 
 As for who would fix it, LGTM mentions which commit caused the failure and 
 who was the author of it. So i think its the author's responsibility to 
 fix it.
 
 Personally, LGTM list a table that shows how many alerts we caused by 
 which author [ https://lgtm.com/projects/g/apache/geode/contributors:java 
  ]
 I cleaning up whatever alerts I have introduced into Apache Geode.
 
 Regards
 Nabarun
 
 
> On Nov 9, 2018, at 2:54 PM, Alexander Murmann  wrote:
> 
> I don't have strong opinions on this, but I am always suspect of CI jobs
> that indicate quality that only run periodically. If the job discovers
> something that needs improvement who is going to do the work and when?
> 
>> On Fri, Nov 9, 2018 at 2:36 PM Kirk Lund  wrote:
>> 
>> Well, we could run it periodically such as weekly rather than as part of
>> the main pipeline or precheckin.
>> 
>> On Fri, Nov 9, 2018 at 2:32 PM, Aditya Anchuri 
>> wrote:
>> 
>>> +1, although I do wonder about the overhead of making PRs increasing 
>>> more
>>> than it already feels like to me as a new contributor (as the person who
>>> made the geospatial contribution). If this was a gradle task maybe like
>>> spotless?
>>> 
>>> On Fri, Nov 9, 2018 at 2:20 PM Bruce Schuchardt 
>>> wrote:
>>> 
 I'd like to see LGTM run on pull requests.  Otherwise I think we're
 fighting a losing battle trying to improve the quality of our code. For
 instance, we just had a nice contribution of geospatial functionality
 that raised 5 alerts, but we only found out about it after the code was
 merged to develop.
 
 LGTM allows that kind of integration but you have to be the repo
>> "owner"
 to set it up.
 
 
 https://lgtm.com/projects/g/apache/geode/

Re: PowerMock and mock ClassLoader

2018-12-05 Thread Alexander Murmann 
+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 
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 
> 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  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 
> > > 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  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 
> 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 
> > 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:6

Re: PowerMock and mock ClassLoader

2018-12-05 Thread Helena Bales 
+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 
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 
> 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 
> > 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  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 
> > > > 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  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 
> > 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 
> > > 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.Ser

Re: PowerMock and mock ClassLoader

2018-12-05 Thread Ryan McMahon 
Makes sense to me - I will pursue removing the PowerMock dependency all
together when all usages are removed.

Ryan

On Wed, Dec 5, 2018 at 2:47 PM Helena Bales  wrote:

> +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 
> 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 
> > 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 
> > > 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 
> 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 
> 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 
> > > 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 
> > > > 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

[VOTE] Apache Geode 1.8.0 RC2

2018-12-05 Thread Alexander Murmann 
Hi Apache Geode community,

Below you find all the information for the the second release candidate of
Geode 1.8.0. All packaging issues related to Geode native should be
resolved in this candidate. Everything else is unchanged.

Please review and provide feedback, so that the vote can end by the end of
day Monday, Dec. 10th.

Release notes can still be found at
https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.8.0

Apache Geode:
https://github.com/apache/geode/tree/rel/v1.8.0.RC2
Apache Geode examples:
https://github.com/apache/geode-examples/tree/rel/v1.8.0.RC2
Apache Geode Native:
https://github.com/apache/geode-native/tree/rel/v1.8.0.RC2

Commit IDs:
Apache Geode: 671671b5e81acde2684df3331aedf176cc927e6e
Apache Geode Examples: aee3794f1302ffab51b4ca5d02657598420b7a00
Apache Geode Native: ae8c6b2500ee8ffb600e5a295c053e9b2ac880ee

Source and binary files:
https://dist.apache.org/repos/dist/dev/geode/1.8.0.RC2/

Maven staging repo:
https://repository.apache.org/content/repositories/orgapachegeode-1048

Geode's KEYS file containing PGP keys we use to sign the release:
https://github.com/apache/geode/blob/develop/KEYS

Signed the release with fingerprint:
rsa4096 2018-09-01 [SC]
D5C5C950D61898EDE8928820D6048392BDFB7797