Re: no test category, rename tests?

2018-08-21 Thread Ryan McMahon
+1 for the naming convention of *Test, *IntegrationTest, and
*DistributedTest.  It’s mostly personal preference, but I also agree with
Kirk’s comment that it makes it easier to find the test you are looking for
by file name.  I also find it easier to see the test type I’m working with
in the file tabs of my IDE without thinking about the source set.  Again,
it’s mostly personal preference but I think it’s a good convention.

Ryan

On Tue, Aug 21, 2018 at 12:00 PM Kirk Lund  wrote:

> Imagine if all the dunits and integration tests were named FooTest and now
> you finally want to write a unit test for Foo. You'll need to create a 3rd
> test named FootTest or rename the existing FooTests to FooIntegrationTest
> or FooDistributedTest. Since we have separate source sets for all three,
> naming all three FooTest is possible. But in my opinion this is just going
> to cause a lot of suffering when you're searching for the right test to
> modify for a code change or to debug a CI failure. What do you prefer?
>
> On Tue, Aug 21, 2018 at 11:51 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com
> > wrote:
>
> > Are there any benefits of naming them specifically to reflect the
> category?
> > I am used to look at test category annotation to differentiate a test is
> a
> > single-vm or a multi-vm.
> >
> > On Tue, Aug 21, 2018 at 11:29 AM Anthony Baker 
> wrote:
> >
> > > The original purpose of specific names was because we filtered test
> > > execution based on the name pattern (then we switched to categories and
> > now
> > > we use source sets / dirs).
> > >
> > > I like the idea of changing *JUnitTest to *Test.  I’m not as excited
> > about
> > > *IntegrationTest and friends.  But I’m willing to go with group
> consensus
> > > on this (aka +0).
> > >
> > > Anthony
> > >
> > >
> > > > On Aug 21, 2018, at 10:11 AM, Kirk Lund  wrote:
> > > >
> > > > +1 Yes! This naming scheme matches what we already have on the Apache
> > > Geode
> > > > Wiki [1] for naming tests.
> > > >
> > > > Side note: Looks like we need to update the pages on the Wiki to
> match
> > > our
> > > > recent changes involving JUnit categories.
> > > >
> > > > [1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests
> > > >
> > > > On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda <
> > > sai_boorlaga...@apache.org
> > > >> wrote:
> > > >
> > > >> Many tests file names are *JUnitTest.java[1] even though it is an
> > > >> integration test or distributed test. After we segregated into
> > > respective
> > > >> folders and removed the test category it is not visible which test
> it
> > is
> > > >> unless we look back into the path of the source directory
> > > (integrationTest
> > > >> or distributedTest).
> > > >>
> > > >> So I wanted to rename tests to reflect test category. Let me know if
> > > there
> > > >> is okay and if I do rename are there any downstream impacts (eg:
> > > metrics,
> > > >> auto diagnosis etc)
> > > >>
> > > >> Any integration test will be renamed as *IntegrationTest.java
> > > >> Any distributed test will be renamed as *DistributedTest.java
> > > >> Any unit test will be renamed as *Test.java.
> > > >>
> > > >> [1]
> > > >> https://github.com/apache/geode/blob/develop/geode-core/
> > > >> src/integrationTest/java/org/apache/geode/internal/cache/
> > > >> ComplexDiskRegionJUnitTest.java
> > > >>
> > >
> > >
> >
>


Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Sai Boorlagadda
If not late, I would also like to include GEODE-5594 (enabling hostname
verification)
and GEODE-5338 (using the default SSL context) into 1.7.0.

PRs for both are open and currently up for review.

On Tue, Aug 21, 2018 at 2:26 PM Dan Smith  wrote:

> I think we do want to wait for GEODE-5615 (DistributedTest OOMEs) and
> GEODE-5601 (AcceptanceTest port conflicts) to be fixed before cutting the
> new 1.7 branch. It would be better if we don't create a release branch from
> a point where we have these systematic issues with our pipeline.
>
> -Dan
>
> On Tue, Aug 21, 2018 at 12:48 PM, Nabarun Nag  wrote:
>
> > I will wait for a day to give all developers sometime to commit any new
> > fixes into develop that is needed  in 1.7.0. Please do let me know if
> there
> > is any concern.
> >
> > Regards
> > Nabarun Nag
> >
> > On Tue, Aug 21, 2018 at 12:42 PM Nabarun Nag  wrote:
> >
> > > Yeah , I can continue with the release manager tasks. My understanding
> > > from Dan's email is that every JIRA that was closed as 1.8 needs to be
> > > changed to 1.7 after we rebase develop over release/1.7 branch.
> > >
> > >
> > > Regards
> > > Nabarun Nag
> > >
> > >
> > > On Tue, Aug 21, 2018 at 11:57 AM Dan Smith  wrote:
> > >
> > >> +1 to updating the 1.7 branch.
> > >>
> > >> There is also a 1.8 version in JIRA, and I think a bunch of things are
> > >> marked as resolved in 1.8. So if you update the release branch you
> > should
> > >> probably update the fixed version on all these issues.
> > >>
> > >> -Dan
> > >>
> > >> On Tue, Aug 21, 2018 at 11:39 AM, Alexander Murmann <
> > amurm...@pivotal.io>
> > >> wrote:
> > >>
> > >> > Hi everyone!
> > >> >
> > >> >
> > >> > We cut this release branch 3 months ago and then the release got
> > >> stalled.
> > >> > Since then we’ve added another 432 commits to develop. We also have
> 83
> > >> > resolved Jira tickets marked as 1.8 and another 91 Jira tickets that
> > are
> > >> > labeled as 1.7, but were resolved after the 1.7 branch was cut .
> > >> >
> > >> >
> > >> > Given all the above, I am proposing to update the release/1.7.0
> branch
> > >> to
> > >> > include everything that’s currently on develop. What are everyone's
> > >> > thoughts on this?
> > >> >
> > >> >
> > >> > Nabarun, you previously volunteered to be the release manager for
> 1.7.
> > >> > Would you still be willing to fill that role if we decide to pick
> this
> > >> back
> > >> > up?
> > >> >
> > >> > On Wed, May 23, 2018 at 9:59 AM, Nabarun Nag 
> wrote:
> > >> >
> > >> > > +1
> > >> > >
> > >> > > On Wed, May 23, 2018 at 9:29 AM Michael Stolz 
> > >> wrote:
> > >> > >
> > >> > > > +1
> > >> > > >
> > >> > > > --
> > >> > > > Mike Stolz
> > >> > > > Principal Engineer, GemFire Product Lead
> > >> > > > Mobile: +1-631-835-4771 <(631)%20835-4771>
> > >> > > > Download the GemFire book here.
> > >> > > > <
> > >> > > > https://content.pivotal.io/ebooks/scaling-data-services-
> > >> > > with-pivotal-gemfire
> > >> > > > >
> > >> > > >
> > >> > > > On Wed, May 23, 2018 at 12:24 PM, Barbara Pruijn <
> > >> bpru...@pivotal.io>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > +1
> > >> > > > >
> > >> > > > > > On May 23, 2018, at 8:33 AM, Joey McAllister <
> > >> > jmcallis...@pivotal.io
> > >> > > >
> > >> > > > > wrote:
> > >> > > > > >
> > >> > > > > > +1 for including these. They are documentation-only changes
> > that
> > >> > are
> > >> > > > > > applicable to 1.7.
> > >> > > > > >
> > >> > > > > > On Wed, May 23, 2018 at 8:24 AM Karen Miller <
> > >> kmil...@apache.org>
> > >> > > > wrote:
> > >> > > > > >
> > >> > > > > >> Geode devs,  I think that my merges of commits for
> GEODE-5071
> > >> and
> > >> > > > > >> GEODE-5242 really
> > >> > > > > >> belong in Geode 1.7.  They just missed making it in before
> > the
> > >> > > release
> > >> > > > > >> branch was cut.  I'm going to
> > >> > > > > >> cherry pick them into the 1.7 release branch.  If anyone
> > >> disagrees
> > >> > > > with
> > >> > > > > >> this, let's discuss why, and we
> > >> > > > > >> can always revert the commits.  Thanks!
> > >> > > > > >>
> > >> > > > > >>
> > >> > > > > >> On Mon, May 21, 2018 at 4:39 PM, Nabarun Nag <
> > n...@apache.org>
> > >> > > wrote:
> > >> > > > > >>
> > >> > > > > >>> Hello Geode dev community,
> > >> > > > > >>>
> > >> > > > > >>> We have created a release branch for Apache Geode 1.7.0 -
> > >> > > > > "release/1.7.0"
> > >> > > > > >>>
> > >> > > > > >>> Please do review and raise any issue with the release
> > branch.
> > >> > > > > >>>
> > >> > > > > >>> If no concerns are raised we will start with voting for
> > >> release
> > >> > > > > candidate
> > >> > > > > >>> within a week.
> > >> > > > > >>>
> > >> > > > > >>> Regards
> > >> > > > > >>> Nabarun Nag
> > >> > > > > >>>
> > >> > > > > >>
> > >> > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
>


[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #1016 was SUCCESSFUL (with 2423 tests)

2018-08-21 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #1016 was successful.
---
Scheduled
2425 tests in total.

https://build.spring.io/browse/SGF-NAG-1016/





--
This message is automatically generated by Atlassian Bamboo

Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Anthony Baker
I most definitely agree!

Anthony


> On Aug 21, 2018, at 2:26 PM, Dan Smith  wrote:
> 
> I think we do want to wait for GEODE-5615 (DistributedTest OOMEs) and
> GEODE-5601 (AcceptanceTest port conflicts) to be fixed before cutting the
> new 1.7 branch. It would be better if we don't create a release branch from
> a point where we have these systematic issues with our pipeline.
> 
> -Dan



Re: spotless fails Apache license header?

2018-08-21 Thread Kirk Lund
I suppose I was using that older format of the Apache license header and
then using spotlessApply 100% before running spotlessCheck which was
reformatting the license header. So even though I was using the older one,
I never ran into the problem until today.

So maybe nothing changed?

But, I still think it's ridiculous that we have spotless configured to
disallow a double-space after sentence terminator.

On Tue, Aug 21, 2018 at 3:39 PM, Kirk Lund  wrote:

> I know it's not a bug in spotless. I think we now have the settings a bit
> too strict.
>
> As of 2-3 weeks ago, I was able to follow the "Setting up IntelliJ"
> process that I documented at https://github.com/gemfire/gemfire (search
> down for "Setting up IntelliJ") without spotless failing. See the format
> of the Apache license header that's pasted into that readme? It has the
> extra spaces, including 2 spaces between sentences.
>
> 2-3 weeks ago, this was working fine. Now it fails spotless, so something
> changed. Maybe the version of spotless that we're using in gradle? Or a
> gradle spotless plugin version changed?
>
> At best, it's laughable that our spotless format now complains about
> correct English syntax in comments and javadocs. At worst, it's evidence
> that our use of spotless is... "a bit too strict" which in my opinion
> should be fixed.
>
> Can you please look into what changed? I haven't had much luck finding it
> yet but I assure you that something did change.
>
> On Tue, Aug 21, 2018 at 2:26 PM, Patrick Rhomberg 
> wrote:
>
>> The only addition with respect to spotless on the 10th was to add the
>> `devBuild` target (which runs `spotlessApply`) and to require that
>> `spotlessApply` would run before `compileJava`, if both were to run in a
>> given build command.
>>
>> Looking at the PR against which these failed, it looks like it might be
>> some disagreement between your IDE's desired format and spotless's.
>> Notably, the new test file header is thinner and has more space padding.
>> I
>> hadn't thought spotless cared about comment blocks, but looking now, it
>> does look like we're consistent everywhere else (within the Java code that
>> spotless targets) on how that header is formatted.
>>
>> So, you know... It's a feature, not a bug?  And we should investigate the
>> discrepancies between the format files in /etc, that is, the
>> Eclipse
>> file spotless uses and the IntelliJ file that is meant to emulate it.
>>
>> On Tue, Aug 21, 2018 at 9:48 AM, Kirk Lund  wrote:
>>
>> > This appears to be caused by changes made to the build around August 10?
>> >
>> > On Tue, Aug 21, 2018 at 9:38 AM, Kirk Lund  wrote:
>> >
>> > > Why is spotless now complaining about correct English? By correct
>> > English,
>> > > I mean having 2 spaces between sentences in javadoc or comments (in
>> this
>> > > case it's the Apache license header):
>> > >
>> > > -·*·the·License.··You·may·obtain·a·copy·of·the·License·at
>> > > +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
>> > >
>> > > Execution failed for task ':geode-core:spotlessJava'.
>> > >  
>> > > > The following files had format violations:
>> > >  
>> > >   geode-core/src/main/java/org/apache/geode/internal/cache/
>> > RegionNameValidation.java
>> > >  
>> > >   @@ -1,12 +1,12 @@
>> > >  
>> > >/*
>> > >  
>> > >·*·Licensed·to·the·Apache·Software·Foundation·(ASF)·
>> > under·one·or·more
>> > >  
>> > >   -·*·contributor·license·agreements.··See·the·NOTICE·
>> > file·distributed·with
>> > >  
>> > >   +·*·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
>> > >  
>> > >   +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
>> > >  
>> > >  

Re: spotless fails Apache license header?

2018-08-21 Thread Kirk Lund
I know it's not a bug in spotless. I think we now have the settings a bit
too strict.

As of 2-3 weeks ago, I was able to follow the "Setting up IntelliJ" process
that I documented at https://github.com/gemfire/gemfire (search down
for "Setting
up IntelliJ") without spotless failing. See the format of the Apache
license header that's pasted into that readme? It has the extra spaces,
including 2 spaces between sentences.

2-3 weeks ago, this was working fine. Now it fails spotless, so something
changed. Maybe the version of spotless that we're using in gradle? Or a
gradle spotless plugin version changed?

At best, it's laughable that our spotless format now complains about
correct English syntax in comments and javadocs. At worst, it's evidence
that our use of spotless is... "a bit too strict" which in my opinion
should be fixed.

Can you please look into what changed? I haven't had much luck finding it
yet but I assure you that something did change.

On Tue, Aug 21, 2018 at 2:26 PM, Patrick Rhomberg 
wrote:

> The only addition with respect to spotless on the 10th was to add the
> `devBuild` target (which runs `spotlessApply`) and to require that
> `spotlessApply` would run before `compileJava`, if both were to run in a
> given build command.
>
> Looking at the PR against which these failed, it looks like it might be
> some disagreement between your IDE's desired format and spotless's.
> Notably, the new test file header is thinner and has more space padding.  I
> hadn't thought spotless cared about comment blocks, but looking now, it
> does look like we're consistent everywhere else (within the Java code that
> spotless targets) on how that header is formatted.
>
> So, you know... It's a feature, not a bug?  And we should investigate the
> discrepancies between the format files in /etc, that is, the Eclipse
> file spotless uses and the IntelliJ file that is meant to emulate it.
>
> On Tue, Aug 21, 2018 at 9:48 AM, Kirk Lund  wrote:
>
> > This appears to be caused by changes made to the build around August 10?
> >
> > On Tue, Aug 21, 2018 at 9:38 AM, Kirk Lund  wrote:
> >
> > > Why is spotless now complaining about correct English? By correct
> > English,
> > > I mean having 2 spaces between sentences in javadoc or comments (in
> this
> > > case it's the Apache license header):
> > >
> > > -·*·the·License.··You·may·obtain·a·copy·of·the·License·at
> > > +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
> > >
> > > Execution failed for task ':geode-core:spotlessJava'.
> > >  
> > > > The following files had format violations:
> > >  
> > >   geode-core/src/main/java/org/apache/geode/internal/cache/
> > RegionNameValidation.java
> > >  
> > >   @@ -1,12 +1,12 @@
> > >  
> > >/*
> > >  
> > >·*·Licensed·to·the·Apache·Software·Foundation·(ASF)·
> > under·one·or·more
> > >  
> > >   -·*·contributor·license·agreements.··See·the·NOTICE·
> > file·distributed·with
> > >  
> > >   +·*·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
> > >  
> > >   +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
> > >  
> > >·*
> > >  
> > >   -·*··http://www.apache.org/licenses/LICENSE-2.0
> > >  
> > >   +·*·http://www.apache.org/licenses/LICENSE-2.0
> > >  
> > >·*
> > >  
> > >·*·Unless·required·by·applicable·law·or·agreed·to·
> > in·writing,·software
> > >  

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Jacob Barrett
Ken and I are paired up to containerize now.

> On Aug 21, 2018, at 2:20 PM, Dan Smith  wrote:
> 
> I see we have a few JIRAs that were filed related to this issue. I think I
> cleaned them up, so whoever is working on fixing you can use this JIRA -
> https://issues.apache.org/jira/browse/GEODE-5601.
> 
> Until this is fixed, let's not create new JIRAs for AcceptanceTest failures.
> 
> -Dan
> 
>> On Tue, Aug 21, 2018 at 1:02 PM, Jacob Barrett  wrote:
>> 
>> Until docker on docker is supported for acceptance tests you can disable
>> the parallelism on forks with -DdunitParallelForks=1 when running
>> acceptanceTest. We can do the same in the CI for now too. :(
>> 
>> The change for the CI can be found in
>> ci/pipelines/shared/variablesomething.yml.
>> 
>> -Jake
>> 
>> 
>>> On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:
>>> 
>>> Actually, it looks like the problem is that we are *not* using docker
>>> containers for the acceptance tests. Check this out, in
>>> gradle/docker.gradle. Since acceptance tests use the default port, this
>>> means the test are guaranteed to be flaky, especially since we are
>> running
>>> them in parallel:
>>> 
>>> //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
>>> //acceptanceTest.configure(dockerConfig)
>>> 
>>> I'm not sure what changed that is causing the tests to fail more often
>> now,
>>> but maybe a test ordering change?
>>> 
>>> -Dan
>>> 
>>> 
>>> 
 On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe  wrote:
 
 
 
> On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> 
> GEODE-5590 would seem to imply that GfshRule does not have an
>> adequate
 safe
> guard? If it spawns a server process which binds to the default
>> server
 port
> and that process persists after the test then we need better
>> tearDown.
> 
 Yes, that does appear to be the case. The current failures are
>> apparently
 due to incomplete
 teardown between tests within a test class.
 
 I am attempting  to reproduce the failures on a consistent basis for
 debugging the problem.
 
 
> Actually I thought we were using Docker to run each AcceptanceTest in
> isolation. Then when the test finishes the Docker instances goes
>> away.
 Did
> we stop using Docker for these?
> 
> On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
 sai.boorlaga...@gmail.com
>> wrote:
> 
>> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on
>> Develop.
>> 
>> DeployWithLargeJarTest -
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> develop/jobs/AcceptanceTest/builds/335
>> PutCommandWithJsonTest -
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> develop/jobs/AcceptanceTest/builds/334
>> 
>> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
>> sai.boorlaga...@gmail.com>
>> wrote:
>> 
>>> The metrics job themselves will be green (as they complete to
>>> success)
>> but
>>> you can expand the get_metrics task output and see that build#20
 started
>>> reporting these failures, so probably these are due to recent
>> changes
 on
>>> develop. I believe these metrics are from develop CI test runs.
>>> 
>>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund 
>> wrote:
>>> 
 Those metrics show AcceptanceTests consistently GREEN. Do these
 metrics
 include test failures from pull request precheckin runs like mine?
>>> Or
>> does
 it just cover CI test runs?
 
 On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
 sai.boorlaga...@gmail.com
> wrote:
 
> Metrics show these started failing recently.
> 
> 
 https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> metrics/jobs/
> GeodeAcceptanceTestMetrics/builds/20
> 
> On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
>>> wrote:
> 
>> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to
>> be
 flaky?
>> 
>> My latest pull request failed with these two failures and all I
>>> did
 was
>> extract LocalRegion.validateRegionName and improve unit testing
>> of
>> RegionNameValidation. No other tests failed for me.
>> 
>>> Task :geode-assembly:acceptanceTest
>> > 19680#L5b60bc1a:619
 
>> > 19680#L5b60bc1a:620
 
>> 
 org.apache.geode.management.internal.cli.commands.
>> PutCommandWithJsonTest
>>> putWithJsonString FAILED
>> > 19680#L5b60bc1a:621
 
>>   org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> > 19680#L5b60bc1a:622
 
>>   at

Re: spotless fails Apache license header?

2018-08-21 Thread Patrick Rhomberg
The only addition with respect to spotless on the 10th was to add the
`devBuild` target (which runs `spotlessApply`) and to require that
`spotlessApply` would run before `compileJava`, if both were to run in a
given build command.

Looking at the PR against which these failed, it looks like it might be
some disagreement between your IDE's desired format and spotless's.
Notably, the new test file header is thinner and has more space padding.  I
hadn't thought spotless cared about comment blocks, but looking now, it
does look like we're consistent everywhere else (within the Java code that
spotless targets) on how that header is formatted.

So, you know... It's a feature, not a bug?  And we should investigate the
discrepancies between the format files in /etc, that is, the Eclipse
file spotless uses and the IntelliJ file that is meant to emulate it.

On Tue, Aug 21, 2018 at 9:48 AM, Kirk Lund  wrote:

> This appears to be caused by changes made to the build around August 10?
>
> On Tue, Aug 21, 2018 at 9:38 AM, Kirk Lund  wrote:
>
> > Why is spotless now complaining about correct English? By correct
> English,
> > I mean having 2 spaces between sentences in javadoc or comments (in this
> > case it's the Apache license header):
> >
> > -·*·the·License.··You·may·obtain·a·copy·of·the·License·at
> > +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
> >
> > Execution failed for task ':geode-core:spotlessJava'.
> >  
> > > The following files had format violations:
> >  
> >   geode-core/src/main/java/org/apache/geode/internal/cache/
> RegionNameValidation.java
> >  
> >   @@ -1,12 +1,12 @@
> >  
> >/*
> >  
> >·*·Licensed·to·the·Apache·Software·Foundation·(ASF)·
> under·one·or·more
> >  
> >   -·*·contributor·license·agreements.··See·the·NOTICE·
> file·distributed·with
> >  
> >   +·*·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
> >  
> >   +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
> >  
> >·*
> >  
> >   -·*··http://www.apache.org/licenses/LICENSE-2.0
> >  
> >   +·*·http://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,
> >
> >
> >
>


Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Dan Smith
I think we do want to wait for GEODE-5615 (DistributedTest OOMEs) and
GEODE-5601 (AcceptanceTest port conflicts) to be fixed before cutting the
new 1.7 branch. It would be better if we don't create a release branch from
a point where we have these systematic issues with our pipeline.

-Dan

On Tue, Aug 21, 2018 at 12:48 PM, Nabarun Nag  wrote:

> I will wait for a day to give all developers sometime to commit any new
> fixes into develop that is needed  in 1.7.0. Please do let me know if there
> is any concern.
>
> Regards
> Nabarun Nag
>
> On Tue, Aug 21, 2018 at 12:42 PM Nabarun Nag  wrote:
>
> > Yeah , I can continue with the release manager tasks. My understanding
> > from Dan's email is that every JIRA that was closed as 1.8 needs to be
> > changed to 1.7 after we rebase develop over release/1.7 branch.
> >
> >
> > Regards
> > Nabarun Nag
> >
> >
> > On Tue, Aug 21, 2018 at 11:57 AM Dan Smith  wrote:
> >
> >> +1 to updating the 1.7 branch.
> >>
> >> There is also a 1.8 version in JIRA, and I think a bunch of things are
> >> marked as resolved in 1.8. So if you update the release branch you
> should
> >> probably update the fixed version on all these issues.
> >>
> >> -Dan
> >>
> >> On Tue, Aug 21, 2018 at 11:39 AM, Alexander Murmann <
> amurm...@pivotal.io>
> >> wrote:
> >>
> >> > Hi everyone!
> >> >
> >> >
> >> > We cut this release branch 3 months ago and then the release got
> >> stalled.
> >> > Since then we’ve added another 432 commits to develop. We also have 83
> >> > resolved Jira tickets marked as 1.8 and another 91 Jira tickets that
> are
> >> > labeled as 1.7, but were resolved after the 1.7 branch was cut .
> >> >
> >> >
> >> > Given all the above, I am proposing to update the release/1.7.0 branch
> >> to
> >> > include everything that’s currently on develop. What are everyone's
> >> > thoughts on this?
> >> >
> >> >
> >> > Nabarun, you previously volunteered to be the release manager for 1.7.
> >> > Would you still be willing to fill that role if we decide to pick this
> >> back
> >> > up?
> >> >
> >> > On Wed, May 23, 2018 at 9:59 AM, Nabarun Nag  wrote:
> >> >
> >> > > +1
> >> > >
> >> > > On Wed, May 23, 2018 at 9:29 AM Michael Stolz 
> >> wrote:
> >> > >
> >> > > > +1
> >> > > >
> >> > > > --
> >> > > > Mike Stolz
> >> > > > Principal Engineer, GemFire Product Lead
> >> > > > Mobile: +1-631-835-4771 <(631)%20835-4771>
> >> > > > Download the GemFire book here.
> >> > > > <
> >> > > > https://content.pivotal.io/ebooks/scaling-data-services-
> >> > > with-pivotal-gemfire
> >> > > > >
> >> > > >
> >> > > > On Wed, May 23, 2018 at 12:24 PM, Barbara Pruijn <
> >> bpru...@pivotal.io>
> >> > > > wrote:
> >> > > >
> >> > > > > +1
> >> > > > >
> >> > > > > > On May 23, 2018, at 8:33 AM, Joey McAllister <
> >> > jmcallis...@pivotal.io
> >> > > >
> >> > > > > wrote:
> >> > > > > >
> >> > > > > > +1 for including these. They are documentation-only changes
> that
> >> > are
> >> > > > > > applicable to 1.7.
> >> > > > > >
> >> > > > > > On Wed, May 23, 2018 at 8:24 AM Karen Miller <
> >> kmil...@apache.org>
> >> > > > wrote:
> >> > > > > >
> >> > > > > >> Geode devs,  I think that my merges of commits for GEODE-5071
> >> and
> >> > > > > >> GEODE-5242 really
> >> > > > > >> belong in Geode 1.7.  They just missed making it in before
> the
> >> > > release
> >> > > > > >> branch was cut.  I'm going to
> >> > > > > >> cherry pick them into the 1.7 release branch.  If anyone
> >> disagrees
> >> > > > with
> >> > > > > >> this, let's discuss why, and we
> >> > > > > >> can always revert the commits.  Thanks!
> >> > > > > >>
> >> > > > > >>
> >> > > > > >> On Mon, May 21, 2018 at 4:39 PM, Nabarun Nag <
> n...@apache.org>
> >> > > wrote:
> >> > > > > >>
> >> > > > > >>> Hello Geode dev community,
> >> > > > > >>>
> >> > > > > >>> We have created a release branch for Apache Geode 1.7.0 -
> >> > > > > "release/1.7.0"
> >> > > > > >>>
> >> > > > > >>> Please do review and raise any issue with the release
> branch.
> >> > > > > >>>
> >> > > > > >>> If no concerns are raised we will start with voting for
> >> release
> >> > > > > candidate
> >> > > > > >>> within a week.
> >> > > > > >>>
> >> > > > > >>> Regards
> >> > > > > >>> Nabarun Nag
> >> > > > > >>>
> >> > > > > >>
> >> > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>


Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Dan Smith
I see we have a few JIRAs that were filed related to this issue. I think I
cleaned them up, so whoever is working on fixing you can use this JIRA -
https://issues.apache.org/jira/browse/GEODE-5601.

Until this is fixed, let's not create new JIRAs for AcceptanceTest failures.

-Dan

On Tue, Aug 21, 2018 at 1:02 PM, Jacob Barrett  wrote:

> Until docker on docker is supported for acceptance tests you can disable
> the parallelism on forks with -DdunitParallelForks=1 when running
> acceptanceTest. We can do the same in the CI for now too. :(
>
> The change for the CI can be found in
> ci/pipelines/shared/variablesomething.yml.
>
> -Jake
>
>
> On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:
>
> > Actually, it looks like the problem is that we are *not* using docker
> > containers for the acceptance tests. Check this out, in
> > gradle/docker.gradle. Since acceptance tests use the default port, this
> > means the test are guaranteed to be flaky, especially since we are
> running
> > them in parallel:
> >
> > //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
> > //acceptanceTest.configure(dockerConfig)
> >
> > I'm not sure what changed that is causing the tests to fail more often
> now,
> > but maybe a test ordering change?
> >
> > -Dan
> >
> >
> >
> > On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe  wrote:
> >
> > >
> > >
> > > > On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> > > >
> > > > GEODE-5590 would seem to imply that GfshRule does not have an
> adequate
> > > safe
> > > > guard? If it spawns a server process which binds to the default
> server
> > > port
> > > > and that process persists after the test then we need better
> tearDown.
> > > >
> > > Yes, that does appear to be the case. The current failures are
> apparently
> > > due to incomplete
> > > teardown between tests within a test class.
> > >
> > > I am attempting  to reproduce the failures on a consistent basis for
> > > debugging the problem.
> > >
> > >
> > > > Actually I thought we were using Docker to run each AcceptanceTest in
> > > > isolation. Then when the test finishes the Docker instances goes
> away.
> > > Did
> > > > we stop using Docker for these?
> > > >
> > > > On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> > > sai.boorlaga...@gmail.com
> > > >> wrote:
> > > >
> > > >> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on
> Develop.
> > > >>
> > > >> DeployWithLargeJarTest -
> > > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > > >> develop/jobs/AcceptanceTest/builds/335
> > > >> PutCommandWithJsonTest -
> > > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > > >> develop/jobs/AcceptanceTest/builds/334
> > > >>
> > > >> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
> > > >> sai.boorlaga...@gmail.com>
> > > >> wrote:
> > > >>
> > > >>> The metrics job themselves will be green (as they complete to
> > success)
> > > >> but
> > > >>> you can expand the get_metrics task output and see that build#20
> > > started
> > > >>> reporting these failures, so probably these are due to recent
> changes
> > > on
> > > >>> develop. I believe these metrics are from develop CI test runs.
> > > >>>
> > > >>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund 
> wrote:
> > > >>>
> > >  Those metrics show AcceptanceTests consistently GREEN. Do these
> > > metrics
> > >  include test failures from pull request precheckin runs like mine?
> > Or
> > > >> does
> > >  it just cover CI test runs?
> > > 
> > >  On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> > >  sai.boorlaga...@gmail.com
> > > > wrote:
> > > 
> > > > Metrics show these started failing recently.
> > > >
> > > >
> > >  https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > > >> metrics/jobs/
> > > > GeodeAcceptanceTestMetrics/builds/20
> > > >
> > > > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
> > wrote:
> > > >
> > > >> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to
> be
> > >  flaky?
> > > >>
> > > >> My latest pull request failed with these two failures and all I
> > did
> > >  was
> > > >> extract LocalRegion.validateRegionName and improve unit testing
> of
> > > >> RegionNameValidation. No other tests failed for me.
> > > >>
> > > >>> Task :geode-assembly:acceptanceTest
> > > >>  19680#L5b60bc1a:619
> > >
> > > >>  19680#L5b60bc1a:620
> > >
> > > >>
> > >  org.apache.geode.management.internal.cli.commands.
> > > >> PutCommandWithJsonTest
> > > >>> putWithJsonString FAILED
> > > >>  19680#L5b60bc1a:621
> > >
> > > >>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > > >>  19680#L5b60bc1a:622
> > >
> > > >>at
> > >  

Build for version 1.8.0-build.1313 of Apache Geode failed.

2018-08-21 Thread apachegeodeci
=

The build job for Apache Geode version 1.8.0-build.1313 has failed.


Build artifacts are available at:
http://files.apachegeode-ci.info/builds/1.8.0-build.1313/geode-build-artifacts-1.8.0-build.1313.tgz

Test results are available at:
http://files.apachegeode-ci.info/builds/1.8.0-build.1313/test-results/build/


Job: 
https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/Build/builds/356

=


Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Jacob Barrett
Until docker on docker is supported for acceptance tests you can disable
the parallelism on forks with -DdunitParallelForks=1 when running
acceptanceTest. We can do the same in the CI for now too. :(

The change for the CI can be found in
ci/pipelines/shared/variablesomething.yml.

-Jake


On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:

> Actually, it looks like the problem is that we are *not* using docker
> containers for the acceptance tests. Check this out, in
> gradle/docker.gradle. Since acceptance tests use the default port, this
> means the test are guaranteed to be flaky, especially since we are running
> them in parallel:
>
> //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
> //acceptanceTest.configure(dockerConfig)
>
> I'm not sure what changed that is causing the tests to fail more often now,
> but maybe a test ordering change?
>
> -Dan
>
>
>
> On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe  wrote:
>
> >
> >
> > > On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> > >
> > > GEODE-5590 would seem to imply that GfshRule does not have an adequate
> > safe
> > > guard? If it spawns a server process which binds to the default server
> > port
> > > and that process persists after the test then we need better tearDown.
> > >
> > Yes, that does appear to be the case. The current failures are apparently
> > due to incomplete
> > teardown between tests within a test class.
> >
> > I am attempting  to reproduce the failures on a consistent basis for
> > debugging the problem.
> >
> >
> > > Actually I thought we were using Docker to run each AcceptanceTest in
> > > isolation. Then when the test finishes the Docker instances goes away.
> > Did
> > > we stop using Docker for these?
> > >
> > > On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> > sai.boorlaga...@gmail.com
> > >> wrote:
> > >
> > >> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
> > >>
> > >> DeployWithLargeJarTest -
> > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > >> develop/jobs/AcceptanceTest/builds/335
> > >> PutCommandWithJsonTest -
> > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > >> develop/jobs/AcceptanceTest/builds/334
> > >>
> > >> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
> > >> sai.boorlaga...@gmail.com>
> > >> wrote:
> > >>
> > >>> The metrics job themselves will be green (as they complete to
> success)
> > >> but
> > >>> you can expand the get_metrics task output and see that build#20
> > started
> > >>> reporting these failures, so probably these are due to recent changes
> > on
> > >>> develop. I believe these metrics are from develop CI test runs.
> > >>>
> > >>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
> > >>>
> >  Those metrics show AcceptanceTests consistently GREEN. Do these
> > metrics
> >  include test failures from pull request precheckin runs like mine?
> Or
> > >> does
> >  it just cover CI test runs?
> > 
> >  On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> >  sai.boorlaga...@gmail.com
> > > wrote:
> > 
> > > Metrics show these started failing recently.
> > >
> > >
> >  https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > >> metrics/jobs/
> > > GeodeAcceptanceTestMetrics/builds/20
> > >
> > > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
> wrote:
> > >
> > >> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
> >  flaky?
> > >>
> > >> My latest pull request failed with these two failures and all I
> did
> >  was
> > >> extract LocalRegion.validateRegionName and improve unit testing of
> > >> RegionNameValidation. No other tests failed for me.
> > >>
> > >>> Task :geode-assembly:acceptanceTest
> > >>  >
> > >>  >
> > >>
> >  org.apache.geode.management.internal.cli.commands.
> > >> PutCommandWithJsonTest
> > >>> putWithJsonString FAILED
> > >>  >
> > >>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > >>  >
> > >>at
> >  sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > >> Method)
> > >>  >
> > >>at
> > >> sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > > NativeConstructorAccessorImpl.java:62)
> > >>  >
> > >>at
> > >> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > > DelegatingConstructorAccessorImpl.java:45)
> > >>  >
> > >>at
> > >> 

Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Nabarun Nag
I will wait for a day to give all developers sometime to commit any new
fixes into develop that is needed  in 1.7.0. Please do let me know if there
is any concern.

Regards
Nabarun Nag

On Tue, Aug 21, 2018 at 12:42 PM Nabarun Nag  wrote:

> Yeah , I can continue with the release manager tasks. My understanding
> from Dan's email is that every JIRA that was closed as 1.8 needs to be
> changed to 1.7 after we rebase develop over release/1.7 branch.
>
>
> Regards
> Nabarun Nag
>
>
> On Tue, Aug 21, 2018 at 11:57 AM Dan Smith  wrote:
>
>> +1 to updating the 1.7 branch.
>>
>> There is also a 1.8 version in JIRA, and I think a bunch of things are
>> marked as resolved in 1.8. So if you update the release branch you should
>> probably update the fixed version on all these issues.
>>
>> -Dan
>>
>> On Tue, Aug 21, 2018 at 11:39 AM, Alexander Murmann 
>> wrote:
>>
>> > Hi everyone!
>> >
>> >
>> > We cut this release branch 3 months ago and then the release got
>> stalled.
>> > Since then we’ve added another 432 commits to develop. We also have 83
>> > resolved Jira tickets marked as 1.8 and another 91 Jira tickets that are
>> > labeled as 1.7, but were resolved after the 1.7 branch was cut .
>> >
>> >
>> > Given all the above, I am proposing to update the release/1.7.0 branch
>> to
>> > include everything that’s currently on develop. What are everyone's
>> > thoughts on this?
>> >
>> >
>> > Nabarun, you previously volunteered to be the release manager for 1.7.
>> > Would you still be willing to fill that role if we decide to pick this
>> back
>> > up?
>> >
>> > On Wed, May 23, 2018 at 9:59 AM, Nabarun Nag  wrote:
>> >
>> > > +1
>> > >
>> > > On Wed, May 23, 2018 at 9:29 AM Michael Stolz 
>> wrote:
>> > >
>> > > > +1
>> > > >
>> > > > --
>> > > > Mike Stolz
>> > > > Principal Engineer, GemFire Product Lead
>> > > > Mobile: +1-631-835-4771 <(631)%20835-4771>
>> > > > Download the GemFire book here.
>> > > > <
>> > > > https://content.pivotal.io/ebooks/scaling-data-services-
>> > > with-pivotal-gemfire
>> > > > >
>> > > >
>> > > > On Wed, May 23, 2018 at 12:24 PM, Barbara Pruijn <
>> bpru...@pivotal.io>
>> > > > wrote:
>> > > >
>> > > > > +1
>> > > > >
>> > > > > > On May 23, 2018, at 8:33 AM, Joey McAllister <
>> > jmcallis...@pivotal.io
>> > > >
>> > > > > wrote:
>> > > > > >
>> > > > > > +1 for including these. They are documentation-only changes that
>> > are
>> > > > > > applicable to 1.7.
>> > > > > >
>> > > > > > On Wed, May 23, 2018 at 8:24 AM Karen Miller <
>> kmil...@apache.org>
>> > > > wrote:
>> > > > > >
>> > > > > >> Geode devs,  I think that my merges of commits for GEODE-5071
>> and
>> > > > > >> GEODE-5242 really
>> > > > > >> belong in Geode 1.7.  They just missed making it in before the
>> > > release
>> > > > > >> branch was cut.  I'm going to
>> > > > > >> cherry pick them into the 1.7 release branch.  If anyone
>> disagrees
>> > > > with
>> > > > > >> this, let's discuss why, and we
>> > > > > >> can always revert the commits.  Thanks!
>> > > > > >>
>> > > > > >>
>> > > > > >> On Mon, May 21, 2018 at 4:39 PM, Nabarun Nag 
>> > > wrote:
>> > > > > >>
>> > > > > >>> Hello Geode dev community,
>> > > > > >>>
>> > > > > >>> We have created a release branch for Apache Geode 1.7.0 -
>> > > > > "release/1.7.0"
>> > > > > >>>
>> > > > > >>> Please do review and raise any issue with the release branch.
>> > > > > >>>
>> > > > > >>> If no concerns are raised we will start with voting for
>> release
>> > > > > candidate
>> > > > > >>> within a week.
>> > > > > >>>
>> > > > > >>> Regards
>> > > > > >>> Nabarun Nag
>> > > > > >>>
>> > > > > >>
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>


Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Nabarun Nag
Yeah , I can continue with the release manager tasks. My understanding from
Dan's email is that every JIRA that was closed as 1.8 needs to be changed
to 1.7 after we rebase develop over release/1.7 branch.


Regards
Nabarun Nag

On Tue, Aug 21, 2018 at 11:57 AM Dan Smith  wrote:

> +1 to updating the 1.7 branch.
>
> There is also a 1.8 version in JIRA, and I think a bunch of things are
> marked as resolved in 1.8. So if you update the release branch you should
> probably update the fixed version on all these issues.
>
> -Dan
>
> On Tue, Aug 21, 2018 at 11:39 AM, Alexander Murmann 
> wrote:
>
> > Hi everyone!
> >
> >
> > We cut this release branch 3 months ago and then the release got stalled.
> > Since then we’ve added another 432 commits to develop. We also have 83
> > resolved Jira tickets marked as 1.8 and another 91 Jira tickets that are
> > labeled as 1.7, but were resolved after the 1.7 branch was cut .
> >
> >
> > Given all the above, I am proposing to update the release/1.7.0 branch to
> > include everything that’s currently on develop. What are everyone's
> > thoughts on this?
> >
> >
> > Nabarun, you previously volunteered to be the release manager for 1.7.
> > Would you still be willing to fill that role if we decide to pick this
> back
> > up?
> >
> > On Wed, May 23, 2018 at 9:59 AM, Nabarun Nag  wrote:
> >
> > > +1
> > >
> > > On Wed, May 23, 2018 at 9:29 AM Michael Stolz 
> wrote:
> > >
> > > > +1
> > > >
> > > > --
> > > > Mike Stolz
> > > > Principal Engineer, GemFire Product Lead
> > > > Mobile: +1-631-835-4771 <(631)%20835-4771>
> > > > Download the GemFire book here.
> > > > <
> > > > https://content.pivotal.io/ebooks/scaling-data-services-
> > > with-pivotal-gemfire
> > > > >
> > > >
> > > > On Wed, May 23, 2018 at 12:24 PM, Barbara Pruijn  >
> > > > wrote:
> > > >
> > > > > +1
> > > > >
> > > > > > On May 23, 2018, at 8:33 AM, Joey McAllister <
> > jmcallis...@pivotal.io
> > > >
> > > > > wrote:
> > > > > >
> > > > > > +1 for including these. They are documentation-only changes that
> > are
> > > > > > applicable to 1.7.
> > > > > >
> > > > > > On Wed, May 23, 2018 at 8:24 AM Karen Miller  >
> > > > wrote:
> > > > > >
> > > > > >> Geode devs,  I think that my merges of commits for GEODE-5071
> and
> > > > > >> GEODE-5242 really
> > > > > >> belong in Geode 1.7.  They just missed making it in before the
> > > release
> > > > > >> branch was cut.  I'm going to
> > > > > >> cherry pick them into the 1.7 release branch.  If anyone
> disagrees
> > > > with
> > > > > >> this, let's discuss why, and we
> > > > > >> can always revert the commits.  Thanks!
> > > > > >>
> > > > > >>
> > > > > >> On Mon, May 21, 2018 at 4:39 PM, Nabarun Nag 
> > > wrote:
> > > > > >>
> > > > > >>> Hello Geode dev community,
> > > > > >>>
> > > > > >>> We have created a release branch for Apache Geode 1.7.0 -
> > > > > "release/1.7.0"
> > > > > >>>
> > > > > >>> Please do review and raise any issue with the release branch.
> > > > > >>>
> > > > > >>> If no concerns are raised we will start with voting for release
> > > > > candidate
> > > > > >>> within a week.
> > > > > >>>
> > > > > >>> Regards
> > > > > >>> Nabarun Nag
> > > > > >>>
> > > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>


Re: no test category, rename tests?

2018-08-21 Thread Kirk Lund
Imagine if all the dunits and integration tests were named FooTest and now
you finally want to write a unit test for Foo. You'll need to create a 3rd
test named FootTest or rename the existing FooTests to FooIntegrationTest
or FooDistributedTest. Since we have separate source sets for all three,
naming all three FooTest is possible. But in my opinion this is just going
to cause a lot of suffering when you're searching for the right test to
modify for a code change or to debug a CI failure. What do you prefer?

On Tue, Aug 21, 2018 at 11:51 AM, Sai Boorlagadda  wrote:

> Are there any benefits of naming them specifically to reflect the category?
> I am used to look at test category annotation to differentiate a test is a
> single-vm or a multi-vm.
>
> On Tue, Aug 21, 2018 at 11:29 AM Anthony Baker  wrote:
>
> > The original purpose of specific names was because we filtered test
> > execution based on the name pattern (then we switched to categories and
> now
> > we use source sets / dirs).
> >
> > I like the idea of changing *JUnitTest to *Test.  I’m not as excited
> about
> > *IntegrationTest and friends.  But I’m willing to go with group consensus
> > on this (aka +0).
> >
> > Anthony
> >
> >
> > > On Aug 21, 2018, at 10:11 AM, Kirk Lund  wrote:
> > >
> > > +1 Yes! This naming scheme matches what we already have on the Apache
> > Geode
> > > Wiki [1] for naming tests.
> > >
> > > Side note: Looks like we need to update the pages on the Wiki to match
> > our
> > > recent changes involving JUnit categories.
> > >
> > > [1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests
> > >
> > > On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda <
> > sai_boorlaga...@apache.org
> > >> wrote:
> > >
> > >> Many tests file names are *JUnitTest.java[1] even though it is an
> > >> integration test or distributed test. After we segregated into
> > respective
> > >> folders and removed the test category it is not visible which test it
> is
> > >> unless we look back into the path of the source directory
> > (integrationTest
> > >> or distributedTest).
> > >>
> > >> So I wanted to rename tests to reflect test category. Let me know if
> > there
> > >> is okay and if I do rename are there any downstream impacts (eg:
> > metrics,
> > >> auto diagnosis etc)
> > >>
> > >> Any integration test will be renamed as *IntegrationTest.java
> > >> Any distributed test will be renamed as *DistributedTest.java
> > >> Any unit test will be renamed as *Test.java.
> > >>
> > >> [1]
> > >> https://github.com/apache/geode/blob/develop/geode-core/
> > >> src/integrationTest/java/org/apache/geode/internal/cache/
> > >> ComplexDiskRegionJUnitTest.java
> > >>
> >
> >
>


Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Dan Smith
+1 to updating the 1.7 branch.

There is also a 1.8 version in JIRA, and I think a bunch of things are
marked as resolved in 1.8. So if you update the release branch you should
probably update the fixed version on all these issues.

-Dan

On Tue, Aug 21, 2018 at 11:39 AM, Alexander Murmann 
wrote:

> Hi everyone!
>
>
> We cut this release branch 3 months ago and then the release got stalled.
> Since then we’ve added another 432 commits to develop. We also have 83
> resolved Jira tickets marked as 1.8 and another 91 Jira tickets that are
> labeled as 1.7, but were resolved after the 1.7 branch was cut .
>
>
> Given all the above, I am proposing to update the release/1.7.0 branch to
> include everything that’s currently on develop. What are everyone's
> thoughts on this?
>
>
> Nabarun, you previously volunteered to be the release manager for 1.7.
> Would you still be willing to fill that role if we decide to pick this back
> up?
>
> On Wed, May 23, 2018 at 9:59 AM, Nabarun Nag  wrote:
>
> > +1
> >
> > On Wed, May 23, 2018 at 9:29 AM Michael Stolz  wrote:
> >
> > > +1
> > >
> > > --
> > > Mike Stolz
> > > Principal Engineer, GemFire Product Lead
> > > Mobile: +1-631-835-4771
> > > Download the GemFire book here.
> > > <
> > > https://content.pivotal.io/ebooks/scaling-data-services-
> > with-pivotal-gemfire
> > > >
> > >
> > > On Wed, May 23, 2018 at 12:24 PM, Barbara Pruijn 
> > > wrote:
> > >
> > > > +1
> > > >
> > > > > On May 23, 2018, at 8:33 AM, Joey McAllister <
> jmcallis...@pivotal.io
> > >
> > > > wrote:
> > > > >
> > > > > +1 for including these. They are documentation-only changes that
> are
> > > > > applicable to 1.7.
> > > > >
> > > > > On Wed, May 23, 2018 at 8:24 AM Karen Miller 
> > > wrote:
> > > > >
> > > > >> Geode devs,  I think that my merges of commits for GEODE-5071 and
> > > > >> GEODE-5242 really
> > > > >> belong in Geode 1.7.  They just missed making it in before the
> > release
> > > > >> branch was cut.  I'm going to
> > > > >> cherry pick them into the 1.7 release branch.  If anyone disagrees
> > > with
> > > > >> this, let's discuss why, and we
> > > > >> can always revert the commits.  Thanks!
> > > > >>
> > > > >>
> > > > >> On Mon, May 21, 2018 at 4:39 PM, Nabarun Nag 
> > wrote:
> > > > >>
> > > > >>> Hello Geode dev community,
> > > > >>>
> > > > >>> We have created a release branch for Apache Geode 1.7.0 -
> > > > "release/1.7.0"
> > > > >>>
> > > > >>> Please do review and raise any issue with the release branch.
> > > > >>>
> > > > >>> If no concerns are raised we will start with voting for release
> > > > candidate
> > > > >>> within a week.
> > > > >>>
> > > > >>> Regards
> > > > >>> Nabarun Nag
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>


Re: no test category, rename tests?

2018-08-21 Thread Sai Boorlagadda
Are there any benefits of naming them specifically to reflect the category?
I am used to look at test category annotation to differentiate a test is a
single-vm or a multi-vm.

On Tue, Aug 21, 2018 at 11:29 AM Anthony Baker  wrote:

> The original purpose of specific names was because we filtered test
> execution based on the name pattern (then we switched to categories and now
> we use source sets / dirs).
>
> I like the idea of changing *JUnitTest to *Test.  I’m not as excited about
> *IntegrationTest and friends.  But I’m willing to go with group consensus
> on this (aka +0).
>
> Anthony
>
>
> > On Aug 21, 2018, at 10:11 AM, Kirk Lund  wrote:
> >
> > +1 Yes! This naming scheme matches what we already have on the Apache
> Geode
> > Wiki [1] for naming tests.
> >
> > Side note: Looks like we need to update the pages on the Wiki to match
> our
> > recent changes involving JUnit categories.
> >
> > [1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests
> >
> > On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda <
> sai_boorlaga...@apache.org
> >> wrote:
> >
> >> Many tests file names are *JUnitTest.java[1] even though it is an
> >> integration test or distributed test. After we segregated into
> respective
> >> folders and removed the test category it is not visible which test it is
> >> unless we look back into the path of the source directory
> (integrationTest
> >> or distributedTest).
> >>
> >> So I wanted to rename tests to reflect test category. Let me know if
> there
> >> is okay and if I do rename are there any downstream impacts (eg:
> metrics,
> >> auto diagnosis etc)
> >>
> >> Any integration test will be renamed as *IntegrationTest.java
> >> Any distributed test will be renamed as *DistributedTest.java
> >> Any unit test will be renamed as *Test.java.
> >>
> >> [1]
> >> https://github.com/apache/geode/blob/develop/geode-core/
> >> src/integrationTest/java/org/apache/geode/internal/cache/
> >> ComplexDiskRegionJUnitTest.java
> >>
>
>


Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kirk Lund
I looked through the gradle files and couldn't figure out where to make the
changes. If anyone else knows, please make the changes or let me know if
you want to pair with me so I can learn.

On Tue, Aug 21, 2018 at 11:32 AM, Anthony Baker  wrote:

> By design, acceptanceTest should be run with defaults (including ports).
> So if they aren’t run in containers they can’t be run in parallel.
>
> Anthony
>
>
> > On Aug 21, 2018, at 11:15 AM, Kirk Lund  wrote:
> >
> > Oops... I mean "orphaned processes"
> >
> > On Tue, Aug 21, 2018 at 11:13 AM, Kirk Lund  wrote:
> >
> >> So, we need to disable running them in parallel and improve the tearDown
> >> to make sure these tests don't leave an orphaned tests behind.
> >>
> >> On Tue, Aug 21, 2018 at 11:11 AM, Sai Boorlagadda <
> >> sai.boorlaga...@gmail.com> wrote:
> >>
> >>> jdbc-connector acceptance tests need docker-in-docker (also
> >>> docker-compose)
> >>> to spin up mysql and postgres.
> >>>
> >>> On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:
> >>>
>  Actually, it looks like the problem is that we are *not* using docker
>  containers for the acceptance tests. Check this out, in
>  gradle/docker.gradle. Since acceptance tests use the default port,
> this
>  means the test are guaranteed to be flaky, especially since we are
> >>> running
>  them in parallel:
> 
>  //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
>  //acceptanceTest.configure(dockerConfig)
> 
>  I'm not sure what changed that is causing the tests to fail more often
> >>> now,
>  but maybe a test ordering change?
> 
>  -Dan
> 
> 
> 
>  On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe 
> >>> wrote:
> 
> >
> >
> >> On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> >>
> >> GEODE-5590 would seem to imply that GfshRule does not have an
> >>> adequate
> > safe
> >> guard? If it spawns a server process which binds to the default
> >>> server
> > port
> >> and that process persists after the test then we need better
> >>> tearDown.
> >>
> > Yes, that does appear to be the case. The current failures are
> >>> apparently
> > due to incomplete
> > teardown between tests within a test class.
> >
> > I am attempting  to reproduce the failures on a consistent basis for
> > debugging the problem.
> >
> >
> >> Actually I thought we were using Docker to run each AcceptanceTest
> >>> in
> >> isolation. Then when the test finishes the Docker instances goes
> >>> away.
> > Did
> >> we stop using Docker for these?
> >>
> >> On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> > sai.boorlaga...@gmail.com
> >>> wrote:
> >>
> >>> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on
> >>> Develop.
> >>>
> >>> DeployWithLargeJarTest -
> >>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> >>> develop/jobs/AcceptanceTest/builds/335
> >>> PutCommandWithJsonTest -
> >>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> >>> develop/jobs/AcceptanceTest/builds/334
> >>>
> >>> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
> >>> sai.boorlaga...@gmail.com>
> >>> wrote:
> >>>
>  The metrics job themselves will be green (as they complete to
>  success)
> >>> but
>  you can expand the get_metrics task output and see that build#20
> > started
>  reporting these failures, so probably these are due to recent
> >>> changes
> > on
>  develop. I believe these metrics are from develop CI test runs.
> 
>  On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund 
> >>> wrote:
> 
> > Those metrics show AcceptanceTests consistently GREEN. Do these
> > metrics
> > include test failures from pull request precheckin runs like
> >>> mine?
>  Or
> >>> does
> > it just cover CI test runs?
> >
> > On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> > sai.boorlaga...@gmail.com
> >> wrote:
> >
> >> Metrics show these started failing recently.
> >>
> >>
> > https://concourse.apachegeode-ci.info/teams/main/pipelines/
> >>> metrics/jobs/
> >> GeodeAcceptanceTestMetrics/builds/20
> >>
> >> On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
>  wrote:
> >>
> >>> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to
> >>> be
> > flaky?
> >>>
> >>> My latest pull request failed with these two failures and all I
>  did
> > was
> >>> extract LocalRegion.validateRegionName and improve unit
> >>> testing of
> >>> RegionNameValidation. No other tests failed for me.
> >>>
>  Task :geode-assembly:acceptanceTest
> >>>  >>> 

Re: [DISCUSS] Apache Geode 1.7.0 release branch created

2018-08-21 Thread Alexander Murmann
Hi everyone!


We cut this release branch 3 months ago and then the release got stalled.
Since then we’ve added another 432 commits to develop. We also have 83
resolved Jira tickets marked as 1.8 and another 91 Jira tickets that are
labeled as 1.7, but were resolved after the 1.7 branch was cut .


Given all the above, I am proposing to update the release/1.7.0 branch to
include everything that’s currently on develop. What are everyone's
thoughts on this?


Nabarun, you previously volunteered to be the release manager for 1.7.
Would you still be willing to fill that role if we decide to pick this back
up?

On Wed, May 23, 2018 at 9:59 AM, Nabarun Nag  wrote:

> +1
>
> On Wed, May 23, 2018 at 9:29 AM Michael Stolz  wrote:
>
> > +1
> >
> > --
> > Mike Stolz
> > Principal Engineer, GemFire Product Lead
> > Mobile: +1-631-835-4771
> > Download the GemFire book here.
> > <
> > https://content.pivotal.io/ebooks/scaling-data-services-
> with-pivotal-gemfire
> > >
> >
> > On Wed, May 23, 2018 at 12:24 PM, Barbara Pruijn 
> > wrote:
> >
> > > +1
> > >
> > > > On May 23, 2018, at 8:33 AM, Joey McAllister  >
> > > wrote:
> > > >
> > > > +1 for including these. They are documentation-only changes that are
> > > > applicable to 1.7.
> > > >
> > > > On Wed, May 23, 2018 at 8:24 AM Karen Miller 
> > wrote:
> > > >
> > > >> Geode devs,  I think that my merges of commits for GEODE-5071 and
> > > >> GEODE-5242 really
> > > >> belong in Geode 1.7.  They just missed making it in before the
> release
> > > >> branch was cut.  I'm going to
> > > >> cherry pick them into the 1.7 release branch.  If anyone disagrees
> > with
> > > >> this, let's discuss why, and we
> > > >> can always revert the commits.  Thanks!
> > > >>
> > > >>
> > > >> On Mon, May 21, 2018 at 4:39 PM, Nabarun Nag 
> wrote:
> > > >>
> > > >>> Hello Geode dev community,
> > > >>>
> > > >>> We have created a release branch for Apache Geode 1.7.0 -
> > > "release/1.7.0"
> > > >>>
> > > >>> Please do review and raise any issue with the release branch.
> > > >>>
> > > >>> If no concerns are raised we will start with voting for release
> > > candidate
> > > >>> within a week.
> > > >>>
> > > >>> Regards
> > > >>> Nabarun Nag
> > > >>>
> > > >>
> > >
> > >
> >
>


Re: no test category, rename tests?

2018-08-21 Thread Kirk Lund
There was already an open discussion about how to name tests recently on
the dev list, and the (newish) content about "Writing tests" on the Wiki
reflects what the group wanted at that time. I think everyone in this
community needs to be more active on the dev list in the future, especially
anyone with an opinion.

On Tue, Aug 21, 2018 at 11:33 AM, Jacob Barrett  wrote:

> Yeah +0 from me for the same reason.
>
> I’d just make them all *Test. Let the grouping speak for itself.
>
> > On Aug 21, 2018, at 11:28 AM, Anthony Baker  wrote:
> >
> > The original purpose of specific names was because we filtered test
> execution based on the name pattern (then we switched to categories and now
> we use source sets / dirs).
> >
> > I like the idea of changing *JUnitTest to *Test.  I’m not as excited
> about *IntegrationTest and friends.  But I’m willing to go with group
> consensus on this (aka +0).
> >
> > Anthony
> >
> >
> >> On Aug 21, 2018, at 10:11 AM, Kirk Lund  wrote:
> >>
> >> +1 Yes! This naming scheme matches what we already have on the Apache
> Geode
> >> Wiki [1] for naming tests.
> >>
> >> Side note: Looks like we need to update the pages on the Wiki to match
> our
> >> recent changes involving JUnit categories.
> >>
> >> [1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests
> >>
> >> On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda <
> sai_boorlaga...@apache.org
> >>> wrote:
> >>
> >>> Many tests file names are *JUnitTest.java[1] even though it is an
> >>> integration test or distributed test. After we segregated into
> respective
> >>> folders and removed the test category it is not visible which test it
> is
> >>> unless we look back into the path of the source directory
> (integrationTest
> >>> or distributedTest).
> >>>
> >>> So I wanted to rename tests to reflect test category. Let me know if
> there
> >>> is okay and if I do rename are there any downstream impacts (eg:
> metrics,
> >>> auto diagnosis etc)
> >>>
> >>> Any integration test will be renamed as *IntegrationTest.java
> >>> Any distributed test will be renamed as *DistributedTest.java
> >>> Any unit test will be renamed as *Test.java.
> >>>
> >>> [1]
> >>> https://github.com/apache/geode/blob/develop/geode-core/
> >>> src/integrationTest/java/org/apache/geode/internal/cache/
> >>> ComplexDiskRegionJUnitTest.java
> >>>
> >
>


Re: no test category, rename tests?

2018-08-21 Thread Jacob Barrett
Yeah +0 from me for the same reason.

I’d just make them all *Test. Let the grouping speak for itself.

> On Aug 21, 2018, at 11:28 AM, Anthony Baker  wrote:
> 
> The original purpose of specific names was because we filtered test execution 
> based on the name pattern (then we switched to categories and now we use 
> source sets / dirs).
> 
> I like the idea of changing *JUnitTest to *Test.  I’m not as excited about 
> *IntegrationTest and friends.  But I’m willing to go with group consensus on 
> this (aka +0).
> 
> Anthony
> 
> 
>> On Aug 21, 2018, at 10:11 AM, Kirk Lund  wrote:
>> 
>> +1 Yes! This naming scheme matches what we already have on the Apache Geode
>> Wiki [1] for naming tests.
>> 
>> Side note: Looks like we need to update the pages on the Wiki to match our
>> recent changes involving JUnit categories.
>> 
>> [1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests
>> 
>> On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda >> wrote:
>> 
>>> Many tests file names are *JUnitTest.java[1] even though it is an
>>> integration test or distributed test. After we segregated into respective
>>> folders and removed the test category it is not visible which test it is
>>> unless we look back into the path of the source directory (integrationTest
>>> or distributedTest).
>>> 
>>> So I wanted to rename tests to reflect test category. Let me know if there
>>> is okay and if I do rename are there any downstream impacts (eg: metrics,
>>> auto diagnosis etc)
>>> 
>>> Any integration test will be renamed as *IntegrationTest.java
>>> Any distributed test will be renamed as *DistributedTest.java
>>> Any unit test will be renamed as *Test.java.
>>> 
>>> [1]
>>> https://github.com/apache/geode/blob/develop/geode-core/
>>> src/integrationTest/java/org/apache/geode/internal/cache/
>>> ComplexDiskRegionJUnitTest.java
>>> 
> 


Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Anthony Baker
By design, acceptanceTest should be run with defaults (including ports).  So if 
they aren’t run in containers they can’t be run in parallel.

Anthony


> On Aug 21, 2018, at 11:15 AM, Kirk Lund  wrote:
> 
> Oops... I mean "orphaned processes"
> 
> On Tue, Aug 21, 2018 at 11:13 AM, Kirk Lund  wrote:
> 
>> So, we need to disable running them in parallel and improve the tearDown
>> to make sure these tests don't leave an orphaned tests behind.
>> 
>> On Tue, Aug 21, 2018 at 11:11 AM, Sai Boorlagadda <
>> sai.boorlaga...@gmail.com> wrote:
>> 
>>> jdbc-connector acceptance tests need docker-in-docker (also
>>> docker-compose)
>>> to spin up mysql and postgres.
>>> 
>>> On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:
>>> 
 Actually, it looks like the problem is that we are *not* using docker
 containers for the acceptance tests. Check this out, in
 gradle/docker.gradle. Since acceptance tests use the default port, this
 means the test are guaranteed to be flaky, especially since we are
>>> running
 them in parallel:
 
 //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
 //acceptanceTest.configure(dockerConfig)
 
 I'm not sure what changed that is causing the tests to fail more often
>>> now,
 but maybe a test ordering change?
 
 -Dan
 
 
 
 On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe 
>>> wrote:
 
> 
> 
>> On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
>> 
>> GEODE-5590 would seem to imply that GfshRule does not have an
>>> adequate
> safe
>> guard? If it spawns a server process which binds to the default
>>> server
> port
>> and that process persists after the test then we need better
>>> tearDown.
>> 
> Yes, that does appear to be the case. The current failures are
>>> apparently
> due to incomplete
> teardown between tests within a test class.
> 
> I am attempting  to reproduce the failures on a consistent basis for
> debugging the problem.
> 
> 
>> Actually I thought we were using Docker to run each AcceptanceTest
>>> in
>> isolation. Then when the test finishes the Docker instances goes
>>> away.
> Did
>> we stop using Docker for these?
>> 
>> On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com
>>> wrote:
>> 
>>> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on
>>> Develop.
>>> 
>>> DeployWithLargeJarTest -
>>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>>> develop/jobs/AcceptanceTest/builds/335
>>> PutCommandWithJsonTest -
>>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>>> develop/jobs/AcceptanceTest/builds/334
>>> 
>>> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
>>> sai.boorlaga...@gmail.com>
>>> wrote:
>>> 
 The metrics job themselves will be green (as they complete to
 success)
>>> but
 you can expand the get_metrics task output and see that build#20
> started
 reporting these failures, so probably these are due to recent
>>> changes
> on
 develop. I believe these metrics are from develop CI test runs.
 
 On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund 
>>> wrote:
 
> Those metrics show AcceptanceTests consistently GREEN. Do these
> metrics
> include test failures from pull request precheckin runs like
>>> mine?
 Or
>>> does
> it just cover CI test runs?
> 
> On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com
>> wrote:
> 
>> Metrics show these started failing recently.
>> 
>> 
> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>>> metrics/jobs/
>> GeodeAcceptanceTestMetrics/builds/20
>> 
>> On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
 wrote:
>> 
>>> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to
>>> be
> flaky?
>>> 
>>> My latest pull request failed with these two failures and all I
 did
> was
>>> extract LocalRegion.validateRegionName and improve unit
>>> testing of
>>> RegionNameValidation. No other tests failed for me.
>>> 
 Task :geode-assembly:acceptanceTest
>>> >> L5b60bc1a:619
> 
>>> >> L5b60bc1a:620
> 
>>> 
> org.apache.geode.management.internal.cli.commands.
>>> PutCommandWithJsonTest
 putWithJsonString FAILED
>>> >> L5b60bc1a:621
> 
>>>   org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>>> >> L5b60bc1a:622

Re: no test category, rename tests?

2018-08-21 Thread Anthony Baker
The original purpose of specific names was because we filtered test execution 
based on the name pattern (then we switched to categories and now we use source 
sets / dirs).

I like the idea of changing *JUnitTest to *Test.  I’m not as excited about 
*IntegrationTest and friends.  But I’m willing to go with group consensus on 
this (aka +0).

Anthony


> On Aug 21, 2018, at 10:11 AM, Kirk Lund  wrote:
> 
> +1 Yes! This naming scheme matches what we already have on the Apache Geode
> Wiki [1] for naming tests.
> 
> Side note: Looks like we need to update the pages on the Wiki to match our
> recent changes involving JUnit categories.
> 
> [1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests
> 
> On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda > wrote:
> 
>> Many tests file names are *JUnitTest.java[1] even though it is an
>> integration test or distributed test. After we segregated into respective
>> folders and removed the test category it is not visible which test it is
>> unless we look back into the path of the source directory (integrationTest
>> or distributedTest).
>> 
>> So I wanted to rename tests to reflect test category. Let me know if there
>> is okay and if I do rename are there any downstream impacts (eg: metrics,
>> auto diagnosis etc)
>> 
>> Any integration test will be renamed as *IntegrationTest.java
>> Any distributed test will be renamed as *DistributedTest.java
>> Any unit test will be renamed as *Test.java.
>> 
>> [1]
>> https://github.com/apache/geode/blob/develop/geode-core/
>> src/integrationTest/java/org/apache/geode/internal/cache/
>> ComplexDiskRegionJUnitTest.java
>> 



Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kirk Lund
Oops... I mean "orphaned processes"

On Tue, Aug 21, 2018 at 11:13 AM, Kirk Lund  wrote:

> So, we need to disable running them in parallel and improve the tearDown
> to make sure these tests don't leave an orphaned tests behind.
>
> On Tue, Aug 21, 2018 at 11:11 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com> wrote:
>
>> jdbc-connector acceptance tests need docker-in-docker (also
>> docker-compose)
>> to spin up mysql and postgres.
>>
>> On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:
>>
>> > Actually, it looks like the problem is that we are *not* using docker
>> > containers for the acceptance tests. Check this out, in
>> > gradle/docker.gradle. Since acceptance tests use the default port, this
>> > means the test are guaranteed to be flaky, especially since we are
>> running
>> > them in parallel:
>> >
>> > //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
>> > //acceptanceTest.configure(dockerConfig)
>> >
>> > I'm not sure what changed that is causing the tests to fail more often
>> now,
>> > but maybe a test ordering change?
>> >
>> > -Dan
>> >
>> >
>> >
>> > On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe 
>> wrote:
>> >
>> > >
>> > >
>> > > > On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
>> > > >
>> > > > GEODE-5590 would seem to imply that GfshRule does not have an
>> adequate
>> > > safe
>> > > > guard? If it spawns a server process which binds to the default
>> server
>> > > port
>> > > > and that process persists after the test then we need better
>> tearDown.
>> > > >
>> > > Yes, that does appear to be the case. The current failures are
>> apparently
>> > > due to incomplete
>> > > teardown between tests within a test class.
>> > >
>> > > I am attempting  to reproduce the failures on a consistent basis for
>> > > debugging the problem.
>> > >
>> > >
>> > > > Actually I thought we were using Docker to run each AcceptanceTest
>> in
>> > > > isolation. Then when the test finishes the Docker instances goes
>> away.
>> > > Did
>> > > > we stop using Docker for these?
>> > > >
>> > > > On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
>> > > sai.boorlaga...@gmail.com
>> > > >> wrote:
>> > > >
>> > > >> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on
>> Develop.
>> > > >>
>> > > >> DeployWithLargeJarTest -
>> > > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> > > >> develop/jobs/AcceptanceTest/builds/335
>> > > >> PutCommandWithJsonTest -
>> > > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> > > >> develop/jobs/AcceptanceTest/builds/334
>> > > >>
>> > > >> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
>> > > >> sai.boorlaga...@gmail.com>
>> > > >> wrote:
>> > > >>
>> > > >>> The metrics job themselves will be green (as they complete to
>> > success)
>> > > >> but
>> > > >>> you can expand the get_metrics task output and see that build#20
>> > > started
>> > > >>> reporting these failures, so probably these are due to recent
>> changes
>> > > on
>> > > >>> develop. I believe these metrics are from develop CI test runs.
>> > > >>>
>> > > >>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund 
>> wrote:
>> > > >>>
>> > >  Those metrics show AcceptanceTests consistently GREEN. Do these
>> > > metrics
>> > >  include test failures from pull request precheckin runs like
>> mine?
>> > Or
>> > > >> does
>> > >  it just cover CI test runs?
>> > > 
>> > >  On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
>> > >  sai.boorlaga...@gmail.com
>> > > > wrote:
>> > > 
>> > > > Metrics show these started failing recently.
>> > > >
>> > > >
>> > >  https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> > > >> metrics/jobs/
>> > > > GeodeAcceptanceTestMetrics/builds/20
>> > > >
>> > > > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
>> > wrote:
>> > > >
>> > > >> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to
>> be
>> > >  flaky?
>> > > >>
>> > > >> My latest pull request failed with these two failures and all I
>> > did
>> > >  was
>> > > >> extract LocalRegion.validateRegionName and improve unit
>> testing of
>> > > >> RegionNameValidation. No other tests failed for me.
>> > > >>
>> > > >>> Task :geode-assembly:acceptanceTest
>> > > >> > L5b60bc1a:619
>> > >
>> > > >> > L5b60bc1a:620
>> > >
>> > > >>
>> > >  org.apache.geode.management.internal.cli.commands.
>> > > >> PutCommandWithJsonTest
>> > > >>> putWithJsonString FAILED
>> > > >> > L5b60bc1a:621
>> > >
>> > > >>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> > > >> > L5b60bc1a:622
>> > >
>> > > >>at
>> > >  sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>> > > >> 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Sai Boorlagadda
jdbc-connector acceptance tests need docker-in-docker (also docker-compose)
to spin up mysql and postgres.

On Tue, Aug 21, 2018 at 11:04 AM Dan Smith  wrote:

> Actually, it looks like the problem is that we are *not* using docker
> containers for the acceptance tests. Check this out, in
> gradle/docker.gradle. Since acceptance tests use the default port, this
> means the test are guaranteed to be flaky, especially since we are running
> them in parallel:
>
> //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
> //acceptanceTest.configure(dockerConfig)
>
> I'm not sure what changed that is causing the tests to fail more often now,
> but maybe a test ordering change?
>
> -Dan
>
>
>
> On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe  wrote:
>
> >
> >
> > > On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> > >
> > > GEODE-5590 would seem to imply that GfshRule does not have an adequate
> > safe
> > > guard? If it spawns a server process which binds to the default server
> > port
> > > and that process persists after the test then we need better tearDown.
> > >
> > Yes, that does appear to be the case. The current failures are apparently
> > due to incomplete
> > teardown between tests within a test class.
> >
> > I am attempting  to reproduce the failures on a consistent basis for
> > debugging the problem.
> >
> >
> > > Actually I thought we were using Docker to run each AcceptanceTest in
> > > isolation. Then when the test finishes the Docker instances goes away.
> > Did
> > > we stop using Docker for these?
> > >
> > > On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> > sai.boorlaga...@gmail.com
> > >> wrote:
> > >
> > >> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
> > >>
> > >> DeployWithLargeJarTest -
> > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > >> develop/jobs/AcceptanceTest/builds/335
> > >> PutCommandWithJsonTest -
> > >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > >> develop/jobs/AcceptanceTest/builds/334
> > >>
> > >> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
> > >> sai.boorlaga...@gmail.com>
> > >> wrote:
> > >>
> > >>> The metrics job themselves will be green (as they complete to
> success)
> > >> but
> > >>> you can expand the get_metrics task output and see that build#20
> > started
> > >>> reporting these failures, so probably these are due to recent changes
> > on
> > >>> develop. I believe these metrics are from develop CI test runs.
> > >>>
> > >>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
> > >>>
> >  Those metrics show AcceptanceTests consistently GREEN. Do these
> > metrics
> >  include test failures from pull request precheckin runs like mine?
> Or
> > >> does
> >  it just cover CI test runs?
> > 
> >  On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> >  sai.boorlaga...@gmail.com
> > > wrote:
> > 
> > > Metrics show these started failing recently.
> > >
> > >
> >  https://concourse.apachegeode-ci.info/teams/main/pipelines/
> > >> metrics/jobs/
> > > GeodeAcceptanceTestMetrics/builds/20
> > >
> > > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund 
> wrote:
> > >
> > >> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
> >  flaky?
> > >>
> > >> My latest pull request failed with these two failures and all I
> did
> >  was
> > >> extract LocalRegion.validateRegionName and improve unit testing of
> > >> RegionNameValidation. No other tests failed for me.
> > >>
> > >>> Task :geode-assembly:acceptanceTest
> > >>  >
> > >>  >
> > >>
> >  org.apache.geode.management.internal.cli.commands.
> > >> PutCommandWithJsonTest
> > >>> putWithJsonString FAILED
> > >>  >
> > >>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > >>  >
> > >>at
> >  sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > >> Method)
> > >>  >
> > >>at
> > >> sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > > NativeConstructorAccessorImpl.java:62)
> > >>  >
> > >>at
> > >> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > > DelegatingConstructorAccessorImpl.java:45)
> > >>  >
> > >>at
> > >> org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > > awaitIfNecessary(GfshScript.java:117)
> > >>  >
> > 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Jacob Barrett
The acceptance tests used to run fine in parallel in the same host. We need 
docker in docker working to containerize them. We would need to add docker to 
the docker image and some other fun stuff. Short term you can make the tests 
run serially. :(

-Jake


> On Aug 21, 2018, at 11:04 AM, Dan Smith  wrote:
> 
> Actually, it looks like the problem is that we are *not* using docker
> containers for the acceptance tests. Check this out, in
> gradle/docker.gradle. Since acceptance tests use the default port, this
> means the test are guaranteed to be flaky, especially since we are running
> them in parallel:
> 
> //ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
> //acceptanceTest.configure(dockerConfig)
> 
> I'm not sure what changed that is causing the tests to fail more often now,
> but maybe a test ordering change?
> 
> -Dan
> 
> 
> 
>> On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe  wrote:
>> 
>> 
>> 
>>> On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
>>> 
>>> GEODE-5590 would seem to imply that GfshRule does not have an adequate
>> safe
>>> guard? If it spawns a server process which binds to the default server
>> port
>>> and that process persists after the test then we need better tearDown.
>>> 
>> Yes, that does appear to be the case. The current failures are apparently
>> due to incomplete
>> teardown between tests within a test class.
>> 
>> I am attempting  to reproduce the failures on a consistent basis for
>> debugging the problem.
>> 
>> 
>>> Actually I thought we were using Docker to run each AcceptanceTest in
>>> isolation. Then when the test finishes the Docker instances goes away.
>> Did
>>> we stop using Docker for these?
>>> 
>>> On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
>> sai.boorlaga...@gmail.com
 wrote:
>>> 
 DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
 
 DeployWithLargeJarTest -
 https://concourse.apachegeode-ci.info/teams/main/pipelines/
 develop/jobs/AcceptanceTest/builds/335
 PutCommandWithJsonTest -
 https://concourse.apachegeode-ci.info/teams/main/pipelines/
 develop/jobs/AcceptanceTest/builds/334
 
 On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
 sai.boorlaga...@gmail.com>
 wrote:
 
> The metrics job themselves will be green (as they complete to success)
 but
> you can expand the get_metrics task output and see that build#20
>> started
> reporting these failures, so probably these are due to recent changes
>> on
> develop. I believe these metrics are from develop CI test runs.
> 
>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
>> 
>> Those metrics show AcceptanceTests consistently GREEN. Do these
>> metrics
>> include test failures from pull request precheckin runs like mine? Or
 does
>> it just cover CI test runs?
>> 
>> On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
>> sai.boorlaga...@gmail.com
>>> wrote:
>> 
>>> Metrics show these started failing recently.
>>> 
>>> 
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
 metrics/jobs/
>>> GeodeAcceptanceTestMetrics/builds/20
>>> 
 On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
 
 Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
>> flaky?
 
 My latest pull request failed with these two failures and all I did
>> was
 extract LocalRegion.validateRegionName and improve unit testing of
 RegionNameValidation. No other tests failed for me.
 
> Task :geode-assembly:acceptanceTest
 
 
 
>> org.apache.geode.management.internal.cli.commands.
 PutCommandWithJsonTest
> putWithJsonString FAILED
 
   org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
 
   at
>> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
 Method)
 
   at
 sun.reflect.NativeConstructorAccessorImpl.newInstance(
>>> NativeConstructorAccessorImpl.java:62)
 
   at
 sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
>>> DelegatingConstructorAccessorImpl.java:45)
 
   at
 org.apache.geode.test.junit.rules.gfsh.GfshScript.
>>> awaitIfNecessary(GfshScript.java:117)
 
   at
 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kirk Lund
A good rule of thumb is to never test with default ports unless the test
runs in a container. Otherwise, running tests in parallel will fail
miserably.

On Tue, Aug 21, 2018 at 10:44 AM, Kirk Lund  wrote:

> GEODE-5590 would seem to imply that GfshRule does not have an adequate
> safe guard? If it spawns a server process which binds to the default server
> port and that process persists after the test then we need better tearDown.
>
> Actually I thought we were using Docker to run each AcceptanceTest in
> isolation. Then when the test finishes the Docker instances goes away. Did
> we stop using Docker for these?
>
> On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com> wrote:
>
>> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
>>
>> DeployWithLargeJarTest -
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/d
>> evelop/jobs/AcceptanceTest/builds/335
>> PutCommandWithJsonTest -
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/d
>> evelop/jobs/AcceptanceTest/builds/334
>>
>> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
>> sai.boorlaga...@gmail.com>
>> wrote:
>>
>> > The metrics job themselves will be green (as they complete to success)
>> but
>> > you can expand the get_metrics task output and see that build#20 started
>> > reporting these failures, so probably these are due to recent changes on
>> > develop. I believe these metrics are from develop CI test runs.
>> >
>> > On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
>> >
>> >> Those metrics show AcceptanceTests consistently GREEN. Do these metrics
>> >> include test failures from pull request precheckin runs like mine? Or
>> does
>> >> it just cover CI test runs?
>> >>
>> >> On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
>> >> sai.boorlaga...@gmail.com
>> >> > wrote:
>> >>
>> >> > Metrics show these started failing recently.
>> >> >
>> >> >
>> >> https://concourse.apachegeode-ci.info/teams/main/pipelines/m
>> etrics/jobs/
>> >> > GeodeAcceptanceTestMetrics/builds/20
>> >> >
>> >> > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
>> >> >
>> >> > > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
>> >> flaky?
>> >> > >
>> >> > > My latest pull request failed with these two failures and all I did
>> >> was
>> >> > > extract LocalRegion.validateRegionName and improve unit testing of
>> >> > > RegionNameValidation. No other tests failed for me.
>> >> > >
>> >> > > > Task :geode-assembly:acceptanceTest
>> >> > >  > >
>> >> > >  > >
>> >> > >
>> >> org.apache.geode.management.internal.cli.commands.PutCommand
>> WithJsonTest
>> >> > > > putWithJsonString FAILED
>> >> > >  > >
>> >> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> >> > >  > >
>> >> > > at
>> >> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>> >> > > Method)
>> >> > >  > >
>> >> > > at
>> >> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
>> >> > NativeConstructorAccessorImpl.java:62)
>> >> > >  > >
>> >> > > at
>> >> > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
>> >> > DelegatingConstructorAccessorImpl.java:45)
>> >> > >  > >
>> >> > > at
>> >> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
>> >> > awaitIfNecessary(GfshScript.java:117)
>> >> > >  > >
>> >> > > at
>> >> > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
>> >> > GfshRule.java:135)
>> >> > >  > >
>> >> > > at
>> >> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
>> >> > GfshScript.java:106)
>> >> > >  > >
>> >> > > at
>> >> > > org.apache.geode.management.internal.cli.commands.
>> >> > PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonT
>> est.java:55)
>> >> > >  > >
>> >> > >  > >
>> >> > >
>> >> org.apache.geode.management.internal.cli.commands.DeployWith
>> LargeJarTest
>> >> > > > deployLargeSetOfJars FAILED
>> >> > >  > >
>> >> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> >> > >  > >
>> >> > > at
>> >> 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Dan Smith
Actually, it looks like the problem is that we are *not* using docker
containers for the acceptance tests. Check this out, in
gradle/docker.gradle. Since acceptance tests use the default port, this
means the test are guaranteed to be flaky, especially since we are running
them in parallel:

//ACCEPTANCE TEST NEEDS DOCKER-COMPOSE TO WORK WITHIN DOCKER FIRST
//acceptanceTest.configure(dockerConfig)

I'm not sure what changed that is causing the tests to fail more often now,
but maybe a test ordering change?

-Dan



On Tue, Aug 21, 2018 at 10:52 AM, Kenneth Howe  wrote:

>
>
> > On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> >
> > GEODE-5590 would seem to imply that GfshRule does not have an adequate
> safe
> > guard? If it spawns a server process which binds to the default server
> port
> > and that process persists after the test then we need better tearDown.
> >
> Yes, that does appear to be the case. The current failures are apparently
> due to incomplete
> teardown between tests within a test class.
>
> I am attempting  to reproduce the failures on a consistent basis for
> debugging the problem.
>
>
> > Actually I thought we were using Docker to run each AcceptanceTest in
> > isolation. Then when the test finishes the Docker instances goes away.
> Did
> > we stop using Docker for these?
> >
> > On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com
> >> wrote:
> >
> >> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
> >>
> >> DeployWithLargeJarTest -
> >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> >> develop/jobs/AcceptanceTest/builds/335
> >> PutCommandWithJsonTest -
> >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> >> develop/jobs/AcceptanceTest/builds/334
> >>
> >> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
> >> sai.boorlaga...@gmail.com>
> >> wrote:
> >>
> >>> The metrics job themselves will be green (as they complete to success)
> >> but
> >>> you can expand the get_metrics task output and see that build#20
> started
> >>> reporting these failures, so probably these are due to recent changes
> on
> >>> develop. I believe these metrics are from develop CI test runs.
> >>>
> >>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
> >>>
>  Those metrics show AcceptanceTests consistently GREEN. Do these
> metrics
>  include test failures from pull request precheckin runs like mine? Or
> >> does
>  it just cover CI test runs?
> 
>  On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
>  sai.boorlaga...@gmail.com
> > wrote:
> 
> > Metrics show these started failing recently.
> >
> >
>  https://concourse.apachegeode-ci.info/teams/main/pipelines/
> >> metrics/jobs/
> > GeodeAcceptanceTestMetrics/builds/20
> >
> > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
> >
> >> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
>  flaky?
> >>
> >> My latest pull request failed with these two failures and all I did
>  was
> >> extract LocalRegion.validateRegionName and improve unit testing of
> >> RegionNameValidation. No other tests failed for me.
> >>
> >>> Task :geode-assembly:acceptanceTest
> >> 
> >> 
> >>
>  org.apache.geode.management.internal.cli.commands.
> >> PutCommandWithJsonTest
> >>> putWithJsonString FAILED
> >> 
> >>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> >> 
> >>at
>  sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> >> Method)
> >> 
> >>at
> >> sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > NativeConstructorAccessorImpl.java:62)
> >> 
> >>at
> >> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > DelegatingConstructorAccessorImpl.java:45)
> >> 
> >>at
> >> org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > awaitIfNecessary(GfshScript.java:117)
> >> 
> >>at
> >> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > GfshRule.java:135)
> >> 
> >>at
> >> org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > GfshScript.java:106)
> >> 
> >>at
> >> 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kenneth Howe



> On Aug 21, 2018, at 10:44 AM, Kirk Lund  wrote:
> 
> GEODE-5590 would seem to imply that GfshRule does not have an adequate safe
> guard? If it spawns a server process which binds to the default server port
> and that process persists after the test then we need better tearDown.
> 
Yes, that does appear to be the case. The current failures are apparently due 
to incomplete 
teardown between tests within a test class.

I am attempting  to reproduce the failures on a consistent basis for debugging 
the problem. 


> Actually I thought we were using Docker to run each AcceptanceTest in
> isolation. Then when the test finishes the Docker instances goes away. Did
> we stop using Docker for these?
> 
> On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda > wrote:
> 
>> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
>> 
>> DeployWithLargeJarTest -
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> develop/jobs/AcceptanceTest/builds/335
>> PutCommandWithJsonTest -
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> develop/jobs/AcceptanceTest/builds/334
>> 
>> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
>> sai.boorlaga...@gmail.com>
>> wrote:
>> 
>>> The metrics job themselves will be green (as they complete to success)
>> but
>>> you can expand the get_metrics task output and see that build#20 started
>>> reporting these failures, so probably these are due to recent changes on
>>> develop. I believe these metrics are from develop CI test runs.
>>> 
>>> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
>>> 
 Those metrics show AcceptanceTests consistently GREEN. Do these metrics
 include test failures from pull request precheckin runs like mine? Or
>> does
 it just cover CI test runs?
 
 On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
 sai.boorlaga...@gmail.com
> wrote:
 
> Metrics show these started failing recently.
> 
> 
 https://concourse.apachegeode-ci.info/teams/main/pipelines/
>> metrics/jobs/
> GeodeAcceptanceTestMetrics/builds/20
> 
> On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
> 
>> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
 flaky?
>> 
>> My latest pull request failed with these two failures and all I did
 was
>> extract LocalRegion.validateRegionName and improve unit testing of
>> RegionNameValidation. No other tests failed for me.
>> 
>>> Task :geode-assembly:acceptanceTest
>> 
>> 
>> 
 org.apache.geode.management.internal.cli.commands.
>> PutCommandWithJsonTest
>>> putWithJsonString FAILED
>> 
>>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> 
>>at
 sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>> Method)
>> 
>>at
>> sun.reflect.NativeConstructorAccessorImpl.newInstance(
> NativeConstructorAccessorImpl.java:62)
>> 
>>at
>> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> DelegatingConstructorAccessorImpl.java:45)
>> 
>>at
>> org.apache.geode.test.junit.rules.gfsh.GfshScript.
> awaitIfNecessary(GfshScript.java:117)
>> 
>>at
>> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> GfshRule.java:135)
>> 
>>at
>> org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> GfshScript.java:106)
>> 
>>at
>> org.apache.geode.management.internal.cli.commands.
> PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:
>> 55)
>> 
>> 
>> 
 org.apache.geode.management.internal.cli.commands.
>> DeployWithLargeJarTest
>>> deployLargeSetOfJars FAILED
>> 
>>org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> 
>>at
 sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>> Method)
>> 
>>at
>> 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kirk Lund
GEODE-5590 would seem to imply that GfshRule does not have an adequate safe
guard? If it spawns a server process which binds to the default server port
and that process persists after the test then we need better tearDown.

Actually I thought we were using Docker to run each AcceptanceTest in
isolation. Then when the test finishes the Docker instances goes away. Did
we stop using Docker for these?

On Tue, Aug 21, 2018 at 10:25 AM, Sai Boorlagadda  wrote:

> DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.
>
> DeployWithLargeJarTest -
> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> develop/jobs/AcceptanceTest/builds/335
> PutCommandWithJsonTest -
> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> develop/jobs/AcceptanceTest/builds/334
>
> On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda <
> sai.boorlaga...@gmail.com>
> wrote:
>
> > The metrics job themselves will be green (as they complete to success)
> but
> > you can expand the get_metrics task output and see that build#20 started
> > reporting these failures, so probably these are due to recent changes on
> > develop. I believe these metrics are from develop CI test runs.
> >
> > On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
> >
> >> Those metrics show AcceptanceTests consistently GREEN. Do these metrics
> >> include test failures from pull request precheckin runs like mine? Or
> does
> >> it just cover CI test runs?
> >>
> >> On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> >> sai.boorlaga...@gmail.com
> >> > wrote:
> >>
> >> > Metrics show these started failing recently.
> >> >
> >> >
> >> https://concourse.apachegeode-ci.info/teams/main/pipelines/
> metrics/jobs/
> >> > GeodeAcceptanceTestMetrics/builds/20
> >> >
> >> > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
> >> >
> >> > > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
> >> flaky?
> >> > >
> >> > > My latest pull request failed with these two failures and all I did
> >> was
> >> > > extract LocalRegion.validateRegionName and improve unit testing of
> >> > > RegionNameValidation. No other tests failed for me.
> >> > >
> >> > > > Task :geode-assembly:acceptanceTest
> >> > >  
> >> > >  
> >> > >
> >> org.apache.geode.management.internal.cli.commands.
> PutCommandWithJsonTest
> >> > > > putWithJsonString FAILED
> >> > >  
> >> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> >> > >  
> >> > > at
> >> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> >> > > Method)
> >> > >  
> >> > > at
> >> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> >> > NativeConstructorAccessorImpl.java:62)
> >> > >  
> >> > > at
> >> > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> >> > DelegatingConstructorAccessorImpl.java:45)
> >> > >  
> >> > > at
> >> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> >> > awaitIfNecessary(GfshScript.java:117)
> >> > >  
> >> > > at
> >> > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> >> > GfshRule.java:135)
> >> > >  
> >> > > at
> >> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> >> > GfshScript.java:106)
> >> > >  
> >> > > at
> >> > > org.apache.geode.management.internal.cli.commands.
> >> > PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:
> 55)
> >> > >  
> >> > >  
> >> > >
> >> org.apache.geode.management.internal.cli.commands.
> DeployWithLargeJarTest
> >> > > > deployLargeSetOfJars FAILED
> >> > >  
> >> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> >> > >  
> >> > > at
> >> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> >> > > Method)
> >> > >  
> >> > > at
> >> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> >> > NativeConstructorAccessorImpl.java:62)
> >> > >  
> >> > > at
> >> > > 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Jens Deppe
Recently all of the AcceptanceTest failures I've looked at have been around
bind exceptions on startup which implies test pollution from prior tests.

--Jens

On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda 
wrote:

> The metrics job themselves will be green (as they complete to success) but
> you can expand the get_metrics task output and see that build#20 started
> reporting these failures, so probably these are due to recent changes on
> develop. I believe these metrics are from develop CI test runs.
>
> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
>
> > Those metrics show AcceptanceTests consistently GREEN. Do these metrics
> > include test failures from pull request precheckin runs like mine? Or
> does
> > it just cover CI test runs?
> >
> > On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> > sai.boorlaga...@gmail.com
> > > wrote:
> >
> > > Metrics show these started failing recently.
> > >
> > >
> https://concourse.apachegeode-ci.info/teams/main/pipelines/metrics/jobs/
> > > GeodeAcceptanceTestMetrics/builds/20
> > >
> > > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
> > >
> > > > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
> > flaky?
> > > >
> > > > My latest pull request failed with these two failures and all I did
> was
> > > > extract LocalRegion.validateRegionName and improve unit testing of
> > > > RegionNameValidation. No other tests failed for me.
> > > >
> > > > > Task :geode-assembly:acceptanceTest
> > > >  
> > > >  
> > > >
> > org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest
> > > > > putWithJsonString FAILED
> > > >  
> > > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > > >  
> > > > at
> > sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > > > Method)
> > > >  
> > > > at
> > > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > > NativeConstructorAccessorImpl.java:62)
> > > >  
> > > > at
> > > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > > DelegatingConstructorAccessorImpl.java:45)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > > awaitIfNecessary(GfshScript.java:117)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > > GfshRule.java:135)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > > GfshScript.java:106)
> > > >  
> > > > at
> > > > org.apache.geode.management.internal.cli.commands.
> > >
> PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:55)
> > > >  
> > > >  
> > > >
> > org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest
> > > > > deployLargeSetOfJars FAILED
> > > >  
> > > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > > >  
> > > > at
> > sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > > > Method)
> > > >  
> > > > at
> > > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > > NativeConstructorAccessorImpl.java:62)
> > > >  
> > > > at
> > > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > > DelegatingConstructorAccessorImpl.java:45)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > > awaitIfNecessary(GfshScript.java:117)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > > GfshRule.java:135)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > > GfshScript.java:106)
> > > >  
> > > > at
> > > > 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kirk Lund
Oh yeah, I see it now! I forgot how to read that page. Thanks!

On Tue, Aug 21, 2018 at 10:18 AM, Sai Boorlagadda  wrote:

> The metrics job themselves will be green (as they complete to success) but
> you can expand the get_metrics task output and see that build#20 started
> reporting these failures, so probably these are due to recent changes on
> develop. I believe these metrics are from develop CI test runs.
>
> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
>
> > Those metrics show AcceptanceTests consistently GREEN. Do these metrics
> > include test failures from pull request precheckin runs like mine? Or
> does
> > it just cover CI test runs?
> >
> > On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> > sai.boorlaga...@gmail.com
> > > wrote:
> >
> > > Metrics show these started failing recently.
> > >
> > > https://concourse.apachegeode-ci.info/teams/main/pipelines/
> metrics/jobs/
> > > GeodeAcceptanceTestMetrics/builds/20
> > >
> > > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
> > >
> > > > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
> > flaky?
> > > >
> > > > My latest pull request failed with these two failures and all I did
> was
> > > > extract LocalRegion.validateRegionName and improve unit testing of
> > > > RegionNameValidation. No other tests failed for me.
> > > >
> > > > > Task :geode-assembly:acceptanceTest
> > > >  
> > > >  
> > > >
> > org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest
> > > > > putWithJsonString FAILED
> > > >  
> > > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > > >  
> > > > at
> > sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > > > Method)
> > > >  
> > > > at
> > > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > > NativeConstructorAccessorImpl.java:62)
> > > >  
> > > > at
> > > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > > DelegatingConstructorAccessorImpl.java:45)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > > awaitIfNecessary(GfshScript.java:117)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > > GfshRule.java:135)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > > GfshScript.java:106)
> > > >  
> > > > at
> > > > org.apache.geode.management.internal.cli.commands.
> > > PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:
> 55)
> > > >  
> > > >  
> > > >
> > org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest
> > > > > deployLargeSetOfJars FAILED
> > > >  
> > > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > > >  
> > > > at
> > sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > > > Method)
> > > >  
> > > > at
> > > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > > NativeConstructorAccessorImpl.java:62)
> > > >  
> > > > at
> > > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > > DelegatingConstructorAccessorImpl.java:45)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > > awaitIfNecessary(GfshScript.java:117)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > > GfshRule.java:135)
> > > >  
> > > > at
> > > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > > GfshScript.java:106)
> > > >  
> > > > at
> > > > org.apache.geode.management.internal.cli.commands.
> > > DeployWithLargeJarTest.deployLargeSetOfJars(
> 

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Sai Boorlagadda
DeployWithLargeJarTest & PutCommandWithJsonTest are flaky on Develop.

DeployWithLargeJarTest -
https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/AcceptanceTest/builds/335
PutCommandWithJsonTest -
https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/AcceptanceTest/builds/334

On Tue, Aug 21, 2018 at 10:18 AM Sai Boorlagadda 
wrote:

> The metrics job themselves will be green (as they complete to success) but
> you can expand the get_metrics task output and see that build#20 started
> reporting these failures, so probably these are due to recent changes on
> develop. I believe these metrics are from develop CI test runs.
>
> On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:
>
>> Those metrics show AcceptanceTests consistently GREEN. Do these metrics
>> include test failures from pull request precheckin runs like mine? Or does
>> it just cover CI test runs?
>>
>> On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
>> sai.boorlaga...@gmail.com
>> > wrote:
>>
>> > Metrics show these started failing recently.
>> >
>> >
>> https://concourse.apachegeode-ci.info/teams/main/pipelines/metrics/jobs/
>> > GeodeAcceptanceTestMetrics/builds/20
>> >
>> > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
>> >
>> > > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
>> flaky?
>> > >
>> > > My latest pull request failed with these two failures and all I did
>> was
>> > > extract LocalRegion.validateRegionName and improve unit testing of
>> > > RegionNameValidation. No other tests failed for me.
>> > >
>> > > > Task :geode-assembly:acceptanceTest
>> > >  
>> > >  
>> > >
>> org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest
>> > > > putWithJsonString FAILED
>> > >  
>> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> > >  
>> > > at
>> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>> > > Method)
>> > >  
>> > > at
>> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
>> > NativeConstructorAccessorImpl.java:62)
>> > >  
>> > > at
>> > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
>> > DelegatingConstructorAccessorImpl.java:45)
>> > >  
>> > > at
>> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
>> > awaitIfNecessary(GfshScript.java:117)
>> > >  
>> > > at
>> > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
>> > GfshRule.java:135)
>> > >  
>> > > at
>> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
>> > GfshScript.java:106)
>> > >  
>> > > at
>> > > org.apache.geode.management.internal.cli.commands.
>> > PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:55)
>> > >  
>> > >  
>> > >
>> org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest
>> > > > deployLargeSetOfJars FAILED
>> > >  
>> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>> > >  
>> > > at
>> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
>> > > Method)
>> > >  
>> > > at
>> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
>> > NativeConstructorAccessorImpl.java:62)
>> > >  
>> > > at
>> > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
>> > DelegatingConstructorAccessorImpl.java:45)
>> > >  
>> > > at
>> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
>> > awaitIfNecessary(GfshScript.java:117)
>> > >  
>> > > at
>> > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
>> > GfshRule.java:135)
>> > >  
>> > > at
>> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
>> > GfshScript.java:106)
>> > >  

Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Sai Boorlagadda
The metrics job themselves will be green (as they complete to success) but
you can expand the get_metrics task output and see that build#20 started
reporting these failures, so probably these are due to recent changes on
develop. I believe these metrics are from develop CI test runs.

On Tue, Aug 21, 2018 at 10:15 AM Kirk Lund  wrote:

> Those metrics show AcceptanceTests consistently GREEN. Do these metrics
> include test failures from pull request precheckin runs like mine? Or does
> it just cover CI test runs?
>
> On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda <
> sai.boorlaga...@gmail.com
> > wrote:
>
> > Metrics show these started failing recently.
> >
> > https://concourse.apachegeode-ci.info/teams/main/pipelines/metrics/jobs/
> > GeodeAcceptanceTestMetrics/builds/20
> >
> > On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
> >
> > > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be
> flaky?
> > >
> > > My latest pull request failed with these two failures and all I did was
> > > extract LocalRegion.validateRegionName and improve unit testing of
> > > RegionNameValidation. No other tests failed for me.
> > >
> > > > Task :geode-assembly:acceptanceTest
> > >  
> > >  
> > >
> org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest
> > > > putWithJsonString FAILED
> > >  
> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > >  
> > > at
> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > > Method)
> > >  
> > > at
> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > NativeConstructorAccessorImpl.java:62)
> > >  
> > > at
> > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > DelegatingConstructorAccessorImpl.java:45)
> > >  
> > > at
> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > awaitIfNecessary(GfshScript.java:117)
> > >  
> > > at
> > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > GfshRule.java:135)
> > >  
> > > at
> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > GfshScript.java:106)
> > >  
> > > at
> > > org.apache.geode.management.internal.cli.commands.
> > PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:55)
> > >  
> > >  
> > >
> org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest
> > > > deployLargeSetOfJars FAILED
> > >  
> > > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> > >  
> > > at
> sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > > Method)
> > >  
> > > at
> > > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> > NativeConstructorAccessorImpl.java:62)
> > >  
> > > at
> > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> > DelegatingConstructorAccessorImpl.java:45)
> > >  
> > > at
> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> > awaitIfNecessary(GfshScript.java:117)
> > >  
> > > at
> > > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> > GfshRule.java:135)
> > >  
> > > at
> > > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> > GfshScript.java:106)
> > >  
> > > at
> > > org.apache.geode.management.internal.cli.commands.
> > DeployWithLargeJarTest.deployLargeSetOfJars(DeployWithLargeJarTest.java:
> > 41)
> > >  
> > >  
> > > > Task :geode-assembly:acceptanceTest FAILED
> > >
> >
>


Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Kirk Lund
Those metrics show AcceptanceTests consistently GREEN. Do these metrics
include test failures from pull request precheckin runs like mine? Or does
it just cover CI test runs?

On Tue, Aug 21, 2018 at 10:09 AM, Sai Boorlagadda  wrote:

> Metrics show these started failing recently.
>
> https://concourse.apachegeode-ci.info/teams/main/pipelines/metrics/jobs/
> GeodeAcceptanceTestMetrics/builds/20
>
> On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:
>
> > Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be flaky?
> >
> > My latest pull request failed with these two failures and all I did was
> > extract LocalRegion.validateRegionName and improve unit testing of
> > RegionNameValidation. No other tests failed for me.
> >
> > > Task :geode-assembly:acceptanceTest
> >  
> >  
> > org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest
> > > putWithJsonString FAILED
> >  
> > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> >  
> > at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > Method)
> >  
> > at
> > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> NativeConstructorAccessorImpl.java:62)
> >  
> > at
> > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> DelegatingConstructorAccessorImpl.java:45)
> >  
> > at
> > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> awaitIfNecessary(GfshScript.java:117)
> >  
> > at
> > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> GfshRule.java:135)
> >  
> > at
> > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> GfshScript.java:106)
> >  
> > at
> > org.apache.geode.management.internal.cli.commands.
> PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:55)
> >  
> >  
> > org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest
> > > deployLargeSetOfJars FAILED
> >  
> > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
> >  
> > at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> > Method)
> >  
> > at
> > sun.reflect.NativeConstructorAccessorImpl.newInstance(
> NativeConstructorAccessorImpl.java:62)
> >  
> > at
> > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(
> DelegatingConstructorAccessorImpl.java:45)
> >  
> > at
> > org.apache.geode.test.junit.rules.gfsh.GfshScript.
> awaitIfNecessary(GfshScript.java:117)
> >  
> > at
> > org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(
> GfshRule.java:135)
> >  
> > at
> > org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(
> GfshScript.java:106)
> >  
> > at
> > org.apache.geode.management.internal.cli.commands.
> DeployWithLargeJarTest.deployLargeSetOfJars(DeployWithLargeJarTest.java:
> 41)
> >  
> >  
> > > Task :geode-assembly:acceptanceTest FAILED
> >
>


Re: no test category, rename tests?

2018-08-21 Thread Kirk Lund
+1 Yes! This naming scheme matches what we already have on the Apache Geode
Wiki [1] for naming tests.

Side note: Looks like we need to update the pages on the Wiki to match our
recent changes involving JUnit categories.

[1] https://cwiki.apache.org/confluence/display/GEODE/Writing+tests

On Tue, Aug 21, 2018 at 9:52 AM, Sai Boorlagadda  wrote:

> Many tests file names are *JUnitTest.java[1] even though it is an
> integration test or distributed test. After we segregated into respective
> folders and removed the test category it is not visible which test it is
> unless we look back into the path of the source directory (integrationTest
> or distributedTest).
>
> So I wanted to rename tests to reflect test category. Let me know if there
> is okay and if I do rename are there any downstream impacts (eg: metrics,
> auto diagnosis etc)
>
> Any integration test will be renamed as *IntegrationTest.java
> Any distributed test will be renamed as *DistributedTest.java
> Any unit test will be renamed as *Test.java.
>
> [1]
> https://github.com/apache/geode/blob/develop/geode-core/
> src/integrationTest/java/org/apache/geode/internal/cache/
> ComplexDiskRegionJUnitTest.java
>


Re: PutCommandWithJsonTest and DeployWithLargeJarTest

2018-08-21 Thread Sai Boorlagadda
Metrics show these started failing recently.

https://concourse.apachegeode-ci.info/teams/main/pipelines/metrics/jobs/GeodeAcceptanceTestMetrics/builds/20

On Tue, Aug 21, 2018 at 10:07 AM Kirk Lund  wrote:

> Are PutCommandWithJsonTest and DeployWithLargeJarTest known to be flaky?
>
> My latest pull request failed with these two failures and all I did was
> extract LocalRegion.validateRegionName and improve unit testing of
> RegionNameValidation. No other tests failed for me.
>
> > Task :geode-assembly:acceptanceTest
>  
>  
> org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest
> > putWithJsonString FAILED
>  
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>  
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> Method)
>  
> at
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
>  
> at
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
>  
> at
> org.apache.geode.test.junit.rules.gfsh.GfshScript.awaitIfNecessary(GfshScript.java:117)
>  
> at
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:135)
>  
> at
> org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(GfshScript.java:106)
>  
> at
> org.apache.geode.management.internal.cli.commands.PutCommandWithJsonTest.putWithJsonString(PutCommandWithJsonTest.java:55)
>  
>  
> org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest
> > deployLargeSetOfJars FAILED
>  
> org.junit.ComparisonFailure: expected:<[0]> but was:<[1]>
>  
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> Method)
>  
> at
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
>  
> at
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
>  
> at
> org.apache.geode.test.junit.rules.gfsh.GfshScript.awaitIfNecessary(GfshScript.java:117)
>  
> at
> org.apache.geode.test.junit.rules.gfsh.GfshRule.execute(GfshRule.java:135)
>  
> at
> org.apache.geode.test.junit.rules.gfsh.GfshScript.execute(GfshScript.java:106)
>  
> at
> org.apache.geode.management.internal.cli.commands.DeployWithLargeJarTest.deployLargeSetOfJars(DeployWithLargeJarTest.java:41)
>  
>  
> > Task :geode-assembly:acceptanceTest FAILED
>


no test category, rename tests?

2018-08-21 Thread Sai Boorlagadda
Many tests file names are *JUnitTest.java[1] even though it is an
integration test or distributed test. After we segregated into respective
folders and removed the test category it is not visible which test it is
unless we look back into the path of the source directory (integrationTest
or distributedTest).

So I wanted to rename tests to reflect test category. Let me know if there
is okay and if I do rename are there any downstream impacts (eg: metrics,
auto diagnosis etc)

Any integration test will be renamed as *IntegrationTest.java
Any distributed test will be renamed as *DistributedTest.java
Any unit test will be renamed as *Test.java.

[1]
https://github.com/apache/geode/blob/develop/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/ComplexDiskRegionJUnitTest.java


Re: spotless fails Apache license header?

2018-08-21 Thread Kirk Lund
This appears to be caused by changes made to the build around August 10?

On Tue, Aug 21, 2018 at 9:38 AM, Kirk Lund  wrote:

> Why is spotless now complaining about correct English? By correct English,
> I mean having 2 spaces between sentences in javadoc or comments (in this
> case it's the Apache license header):
>
> -·*·the·License.··You·may·obtain·a·copy·of·the·License·at
> +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
>
> Execution failed for task ':geode-core:spotlessJava'.
>  
> > The following files had format violations:
>  
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java
>  
>   @@ -1,12 +1,12 @@
>  
>/*
>  
>
> ·*·Licensed·to·the·Apache·Software·Foundation·(ASF)·under·one·or·more
>  
>   
> -·*·contributor·license·agreements.··See·the·NOTICE·file·distributed·with
>  
>   
> +·*·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
>  
>   +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
>  
>·*
>  
>   -·*··http://www.apache.org/licenses/LICENSE-2.0
>  
>   +·*·http://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,
>
>
>


spotless fails Apache license header?

2018-08-21 Thread Kirk Lund
Why is spotless now complaining about correct English? By correct English,
I mean having 2 spaces between sentences in javadoc or comments (in this
case it's the Apache license header):

-·*·the·License.··You·may·obtain·a·copy·of·the·License·at
+·*·the·License.·You·may·obtain·a·copy·of·the·License·at

Execution failed for task ':geode-core:spotlessJava'.
 
> The following files had format violations:
 
  
geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java
 
  @@ -1,12 +1,12 @@
 
   /*
 
   ·*·Licensed·to·the·Apache·Software·Foundation·(ASF)·under·one·or·more
 
  
-·*·contributor·license·agreements.··See·the·NOTICE·file·distributed·with
 
  
+·*·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
 
  +·*·the·License.·You·may·obtain·a·copy·of·the·License·at
 
   ·*
 
  -·*··http://www.apache.org/licenses/LICENSE-2.0
 
  +·*·http://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,