How to setup IntelliJ

[Kirk Lund]

Does anyone have any instructions on setting up IntelliJ with the latest
changes? I’ve tried new project from sources and I’ve tried using ./gradlew
idea — neither works for me. Basically all non-unit tests end up not being
a source set.

Re: How to setup IntelliJ

[Anthony Baker]

Are you using this commit?

commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
Author: Jacob Barrett 
Date:   Tue Jul 17 17:03:22 2018 -0700

GEODE-5363: Fixes issue with build in IJ IDEA.

- Splits up LuceneTestUtilities to removed duplication and compile fail.

Anthony


> On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
> 
> Does anyone have any instructions on setting up IntelliJ with the latest
> changes? I’ve tried new project from sources and I’ve tried using ./gradlew
> idea — neither works for me. Basically all non-unit tests end up not being
> a source set.

Re: How to setup IntelliJ

[Kirk Lund]

Yes, my head revision is...

commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
*origin/develop*, *origin/HEAD*)

Author: Anthony Baker 

Date:   Mon Jul 16 16:56:03 2018 -0700


GEODE-5421 Updated dependencies



Updated bundled library dependencies. The updated libaries are:



HikariCP 3.0.0 -> 3.2.0

fast-classpath-scanner 2.19.0 -> 2.21

fastutil 8.1.1 -> 8.2.1

google-gson 2.8.2 -> 2.8.5

guava 24.1-jre -> 25.1-jre

jackson 2.9.5 -> 2.9.6

jansi 1.17 -> 1.17.1

netty 4.1.21.Final -> 4.1.27.Final

protobuf 3.5.1 -> 3.6.0

spring-security 4.2.4 -> 4.2.7

spring-* 4.3.14 -> 4.3.18

springfox 2.8.0 -> 2.9.2



These changes mean that javassist and reflections are no longer

bundled with the binary convenience distributions.




On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker  wrote:

> Are you using this commit?
>
> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
> Author: Jacob Barrett 
> Date:   Tue Jul 17 17:03:22 2018 -0700
>
> GEODE-5363: Fixes issue with build in IJ IDEA.
>
> - Splits up LuceneTestUtilities to removed duplication and compile
> fail.
>
> Anthony
>
>
> > On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
> >
> > Does anyone have any instructions on setting up IntelliJ with the latest
> > changes? I’ve tried new project from sources and I’ve tried using
> ./gradlew
> > idea — neither works for me. Basically all non-unit tests end up not
> being
> > a source set.
>
>

Re: How to setup IntelliJ

[Kirk Lund]

More details: If I open up BasicDistributedTest, I can run it but all of
the imports for classes that are also in distributedTest are RED and don't
show up in the Project window.

After pulling, the following commit is now my head revision. Unfortunately,
it doesn't fix my problem. But it did cause one change: If I search
for BasicDistributedTest,
it now finds two instances -- one is found in geode-core_distributedTest
and the other is found in geode-core_test.

commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
*origin/develop*)

Author: Jinmei Liao 

Date:   Wed Jul 18 10:22:16 2018 -0700


GEODE-5363: Fixes issue with build in IJ IDEA.


On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund  wrote:

> Yes, my head revision is...
>
> commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
> *origin/develop*, *origin/HEAD*)
>
> Author: Anthony Baker 
>
> Date:   Mon Jul 16 16:56:03 2018 -0700
>
>
> GEODE-5421 Updated dependencies
>
>
>
> Updated bundled library dependencies. The updated libaries are:
>
>
>
> HikariCP 3.0.0 -> 3.2.0
>
> fast-classpath-scanner 2.19.0 -> 2.21
>
> fastutil 8.1.1 -> 8.2.1
>
> google-gson 2.8.2 -> 2.8.5
>
> guava 24.1-jre -> 25.1-jre
>
> jackson 2.9.5 -> 2.9.6
>
> jansi 1.17 -> 1.17.1
>
> netty 4.1.21.Final -> 4.1.27.Final
>
> protobuf 3.5.1 -> 3.6.0
>
> spring-security 4.2.4 -> 4.2.7
>
> spring-* 4.3.14 -> 4.3.18
>
> springfox 2.8.0 -> 2.9.2
>
>
>
> These changes mean that javassist and reflections are no longer
>
> bundled with the binary convenience distributions.
>
>
>
>
> On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker  wrote:
>
>> Are you using this commit?
>>
>> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
>> Author: Jacob Barrett 
>> Date:   Tue Jul 17 17:03:22 2018 -0700
>>
>> GEODE-5363: Fixes issue with build in IJ IDEA.
>>
>> - Splits up LuceneTestUtilities to removed duplication and compile
>> fail.
>>
>> Anthony
>>
>>
>> > On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
>> >
>> > Does anyone have any instructions on setting up IntelliJ with the latest
>> > changes? I’ve tried new project from sources and I’ve tried using
>> ./gradlew
>> > idea — neither works for me. Basically all non-unit tests end up not
>> being
>> > a source set.
>>
>>
>

Moving from gradle to maven

[Kirk Lund]

Gradle appears to not play well with IntelliJ unless the project is overly
simple. I don't want to spend my days fighting with tools that don't work
well together.

Here's an interesting blog article about moving from gradle to maven:

https://blog.philipphauer.de/moving-back-from-gradle-to-maven/

Any other data points or opinions about moving from gradle to maven?

Geode unit tests completed in 'develop/UITests' with non-zero exit code

[apachegeodeci]

Pipeline results can be found at:

Concourse: 
https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/UITests/builds/172

Proposed API for a ConcurrencyTestHelper

[Helena Bales]

Hello All,

Below I have included a proposed API for a concurrency test helper. The
goal of this test helper is to make it easier for tests to use multiple
threads. I believe providing a helper for this will reduce the number of
tests that incorrectly use threads, and help make our DUnit tests more
readable.

I would like feedback on the API below. I have provided the method
signatures that I plan on implementing, as well as an example usage. Please
let me know if there are any methods that seem unnecessary or if there are
any important use cases that I missed here.

Thank you,
Helena Bales

public class ConcurrencyTestHelper {
  private List> toRun;
  private Integer timeout;

  private final Integer DEFAULT_TIMEOUT = 30;

  /**
   * A default constructor that sets the timeout to a default of 30 seconds
   */
  public ConcurrencyTestHelper() {
toRun = new ArrayList>();
timeout = DEFAULT_TIMEOUT;
  }

  /**
   * Run the given Runnable and assert that an exception of the given
type is thrown. The thread
   * will be cancelled when the class timeout is reached.
   * @param runnable a Runnable that throws an exception and responds
to cancellation signal
   * @param exceptionType a Class of Exception that is expected to be
thrown by the runnable
   */
  public void runAndExpectException(Runnable runnable, Class exceptionType);

  /**
   * Run the given Runnable and assert that no exceptions are thrown.
The thread will be cancelled
   * when the class timeout is reached.
   * @param runnable a Runnable that does not throw an exception and
responds to cancellation signal
   */
  public void runAndExpectNoException(Runnable runnable);

  /**
   * Run the given Callable and assert that the result of that
callable is the given expected value.
   * The thread will be cancelled when the class timeout is reached.
   * @param callable a Callable that returns an object, does not throw
exceptions and responds to
   * cancellation signal
   * @param value an Object that is the expected return value from the Callable.
   */
  public void runAndExpectValue(Callable callable, Object value);

  /**
   * Run the given runnable in a loop until the rest of the threads
complete. Fails if the runnable
   * throws an exception during any iteration. After the rest of the
threads complete the loop will
   * not restart the runnable. The running iteration will be cancelled
when the timeout set on the
   * class is reached. A cancellation will result in a test failure.
   * @param runnable a Runnable that does not throw an exception and
responds to cancellation signal
   */
  public void repeatUntilAllThreadsComplete(Runnable runnable);

  /**
   * Run the given runnable in a loop for some number of iterations.
Fails if the runnable throws an
   * exception in any iteration. After the number of iterations has
been met, the loop will not
   * restart the runnable. The running iteration of the Runnable will
be cancelled when the
   * timeout set on the class is reached, regardless of if the number
of desired iterations has
   * been reached. A cancellation will result in a test failure.
   * @param runnable a Runnable that does not throw an exception and
responds to cancellation signal
   * @param iterations a maximum number of iterations to repeat for
   */
  public void repeatForIterations(Runnable runnable, Integer iterations);

  /**
   * Run the given runnable in a loop for a certain duration. Fails if
the runnable throws an
   * exception in any iteration. After the duration has been exceeded,
the loop will not restart
   * the runnable, however the current iteration will not be cancelled
until the timeout value for
   * the class has been reached. A cancellation will result in a test failure.
   * @param runnable a Runnable that does not throw exceptions and
responds to cancellation signal
   * @param duration a maximum amount of time to repeat for
   */
  public void repeatForDuration(Runnable runnable, Integer duration);

  /**
   * Set the timeout for the threads. After the timeout is reached,
the threads will be interrupted
   * and will throw a CancellationException
   */
  public void setTimeout(Integer seconds) {
timeout = seconds;
  }

  /**
   * Runs all callables in toRun in parallel and fail if threads'
conditions were not met. Runs
   * until timeout is reached.
   * @throws InterruptedException if interrupted before timeout
   */
  public void executeInParallel();

  /**
   * Runs all callables in toRun in the order that they were added and
fail if threads' conditions
   * were not met. Runs until timeout is reached.
   * @throws InterruptedException if interrupted before timeout
   */
  public void executeInSeries();
}

// example usage:
new ConcurrentTestHelper()
   .runAndExpectException(runnable, UnsupportedOperationException.class)
   .runAndExpectNoException(runnable)
   .runAndExpectValue(callable, result)
   .repeatUntilAllOtherRunnablesFinish(runnable)
   .repeatFor(runnable, t

Re: Moving from gradle to maven

[Udo Kohlmeyer]

I must agree, the fact that IntelliJ cannot handle the current project 
structure, is that I believe that we have a complicated project structure.

Moving to maven would force a more strict project structure.

I don't mind moving to maven, but I believe that we would have similar 
experiences with maven and a complex project structure. I was thinking 
would could move to Gradle-Kotlin DSL, but that also would not solve the 
current structure problem.


So...  +1 on move to maven OR +1 on refactoring to the current gradle 
setup to be less "custom" and maybe a little more rigid.


On 7/18/18 11:00, Kirk Lund wrote:

Gradle appears to not play well with IntelliJ unless the project is overly
simple. I don't want to spend my days fighting with tools that don't work
well together.

Here's an interesting blog article about moving from gradle to maven:

https://blog.philipphauer.de/moving-back-from-gradle-to-maven/

Any other data points or opinions about moving from gradle to maven?

Re: Proposed API for a ConcurrencyTestHelper

[Dale Emery]

Hi Helena,

I like it!

A few suggestions:

- For the durations (repeatForDuration() and setTimeout()), consider using a 
java.time.Duration rather than an int to express the duration. The Duration 
class lets most of the code not have to keep track of the time units. The 
caller creates the Duration using whatever time unit they want. When some other 
code needs to convert it to a specific time unit, there are methods for that. 
The rest of the code doesn’t have to care about the time units. And there are 
methods for adding, subtracting, and comparing Durations and Instants, all 
handling the time units nicely.

- Consider adding a version of runAndExpectValue that takes a Predicate instead 
of a value. And maybe one taking a Hamcrest Matcher.

- Consider specifying the type of the Callable, and preferably for the value 
passed to runAndExpectValue()). And definitely for Predicates and Matchers, if 
you add those methods.

Cheers,
Dale

> On Jul 18, 2018, at 11:53 AM, Helena Bales  wrote:
> 
> Hello All,
> 
> Below I have included a proposed API for a concurrency test helper. The
> goal of this test helper is to make it easier for tests to use multiple
> threads. I believe providing a helper for this will reduce the number of
> tests that incorrectly use threads, and help make our DUnit tests more
> readable.
> 
> I would like feedback on the API below. I have provided the method
> signatures that I plan on implementing, as well as an example usage. Please
> let me know if there are any methods that seem unnecessary or if there are
> any important use cases that I missed here.
> 
> Thank you,
> Helena Bales
> 
> public class ConcurrencyTestHelper {
>  private List> toRun;
>  private Integer timeout;
> 
>  private final Integer DEFAULT_TIMEOUT = 30;
> 
>  /**
>   * A default constructor that sets the timeout to a default of 30 seconds
>   */
>  public ConcurrencyTestHelper() {
>toRun = new ArrayList>();
>timeout = DEFAULT_TIMEOUT;
>  }
> 
>  /**
>   * Run the given Runnable and assert that an exception of the given
> type is thrown. The thread
>   * will be cancelled when the class timeout is reached.
>   * @param runnable a Runnable that throws an exception and responds
> to cancellation signal
>   * @param exceptionType a Class of Exception that is expected to be
> thrown by the runnable
>   */
>  public void runAndExpectException(Runnable runnable, Class exceptionType);
> 
>  /**
>   * Run the given Runnable and assert that no exceptions are thrown.
> The thread will be cancelled
>   * when the class timeout is reached.
>   * @param runnable a Runnable that does not throw an exception and
> responds to cancellation signal
>   */
>  public void runAndExpectNoException(Runnable runnable);
> 
>  /**
>   * Run the given Callable and assert that the result of that
> callable is the given expected value.
>   * The thread will be cancelled when the class timeout is reached.
>   * @param callable a Callable that returns an object, does not throw
> exceptions and responds to
>   * cancellation signal
>   * @param value an Object that is the expected return value from the 
> Callable.
>   */
>  public void runAndExpectValue(Callable callable, Object value);
> 
>  /**
>   * Run the given runnable in a loop until the rest of the threads
> complete. Fails if the runnable
>   * throws an exception during any iteration. After the rest of the
> threads complete the loop will
>   * not restart the runnable. The running iteration will be cancelled
> when the timeout set on the
>   * class is reached. A cancellation will result in a test failure.
>   * @param runnable a Runnable that does not throw an exception and
> responds to cancellation signal
>   */
>  public void repeatUntilAllThreadsComplete(Runnable runnable);
> 
>  /**
>   * Run the given runnable in a loop for some number of iterations.
> Fails if the runnable throws an
>   * exception in any iteration. After the number of iterations has
> been met, the loop will not
>   * restart the runnable. The running iteration of the Runnable will
> be cancelled when the
>   * timeout set on the class is reached, regardless of if the number
> of desired iterations has
>   * been reached. A cancellation will result in a test failure.
>   * @param runnable a Runnable that does not throw an exception and
> responds to cancellation signal
>   * @param iterations a maximum number of iterations to repeat for
>   */
>  public void repeatForIterations(Runnable runnable, Integer iterations);
> 
>  /**
>   * Run the given runnable in a loop for a certain duration. Fails if
> the runnable throws an
>   * exception in any iteration. After the duration has been exceeded,
> the loop will not restart
>   * the runnable, however the current iteration will not be cancelled
> until the timeout value for
>   * the class has been reached. A cancellation will result in a test failure.
>   * @param runnable a Runnable that does not throw exceptions and
> responds to cancellation signa

Re: How to setup IntelliJ

[Jacob Barrett]

All,

I have a fix that appears to address all these issue in a PR. I am just waiting 
for them to pass come CI because they effect the Gradle build too. Please don’t 
check in any “fixes” to this IJ integration issue.

-Jake


> On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> 
> More details: If I open up BasicDistributedTest, I can run it but all of
> the imports for classes that are also in distributedTest are RED and don't
> show up in the Project window.
> 
> After pulling, the following commit is now my head revision. Unfortunately,
> it doesn't fix my problem. But it did cause one change: If I search
> for BasicDistributedTest,
> it now finds two instances -- one is found in geode-core_distributedTest
> and the other is found in geode-core_test.
> 
> commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
> *origin/develop*)
> 
> Author: Jinmei Liao 
> 
> Date:   Wed Jul 18 10:22:16 2018 -0700
> 
> 
>GEODE-5363: Fixes issue with build in IJ IDEA.
> 
> 
>> On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund  wrote:
>> 
>> Yes, my head revision is...
>> 
>> commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
>> *origin/develop*, *origin/HEAD*)
>> 
>> Author: Anthony Baker 
>> 
>> Date:   Mon Jul 16 16:56:03 2018 -0700
>> 
>> 
>>GEODE-5421 Updated dependencies
>> 
>> 
>> 
>>Updated bundled library dependencies. The updated libaries are:
>> 
>> 
>> 
>>HikariCP 3.0.0 -> 3.2.0
>> 
>>fast-classpath-scanner 2.19.0 -> 2.21
>> 
>>fastutil 8.1.1 -> 8.2.1
>> 
>>google-gson 2.8.2 -> 2.8.5
>> 
>>guava 24.1-jre -> 25.1-jre
>> 
>>jackson 2.9.5 -> 2.9.6
>> 
>>jansi 1.17 -> 1.17.1
>> 
>>netty 4.1.21.Final -> 4.1.27.Final
>> 
>>protobuf 3.5.1 -> 3.6.0
>> 
>>spring-security 4.2.4 -> 4.2.7
>> 
>>spring-* 4.3.14 -> 4.3.18
>> 
>>springfox 2.8.0 -> 2.9.2
>> 
>> 
>> 
>>These changes mean that javassist and reflections are no longer
>> 
>>bundled with the binary convenience distributions.
>> 
>> 
>> 
>> 
>>> On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker  wrote:
>>> 
>>> Are you using this commit?
>>> 
>>> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
>>> Author: Jacob Barrett 
>>> Date:   Tue Jul 17 17:03:22 2018 -0700
>>> 
>>>GEODE-5363: Fixes issue with build in IJ IDEA.
>>> 
>>>- Splits up LuceneTestUtilities to removed duplication and compile
>>> fail.
>>> 
>>> Anthony
>>> 
>>> 
 On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
 
 Does anyone have any instructions on setting up IntelliJ with the latest
 changes? I’ve tried new project from sources and I’ve tried using
>>> ./gradlew
 idea — neither works for me. Basically all non-unit tests end up not
>>> being
 a source set.
>>> 
>>> 
>> 

Re: Moving from gradle to maven

[Patrick Rhomberg]

+1 to correcting our current broken gradle build.


The fault, dear Brutus, is not in our [tools], but in ourselves.

I think the root pain point is that our dependency tree is neither explicit
nor correct in several places.  I have myself had frequent issues
surrounding our Protobuf and OQLLexer classes requiring a command-line
build and re-import.  It's also why, after we bumped gradle versions, we
were prone to errors when building in parallel.

Correctly documenting and making explicit the gradle build dependencies is
something that I am meaning to look into soon, but I am currently looking
into improving our pipelines and metrics scripting.

On Wed, Jul 18, 2018 at 12:04 PM, Udo Kohlmeyer  wrote:

> I must agree, the fact that IntelliJ cannot handle the current project
> structure, is that I believe that we have a complicated project structure.
> Moving to maven would force a more strict project structure.
>
> I don't mind moving to maven, but I believe that we would have similar
> experiences with maven and a complex project structure. I was thinking
> would could move to Gradle-Kotlin DSL, but that also would not solve the
> current structure problem.
>
> So...  +1 on move to maven OR +1 on refactoring to the current gradle
> setup to be less "custom" and maybe a little more rigid.
>
>
> On 7/18/18 11:00, Kirk Lund wrote:
>
>> Gradle appears to not play well with IntelliJ unless the project is overly
>> simple. I don't want to spend my days fighting with tools that don't work
>> well together.
>>
>> Here's an interesting blog article about moving from gradle to maven:
>>
>> https://blog.philipphauer.de/moving-back-from-gradle-to-maven/
>>
>> Any other data points or opinions about moving from gradle to maven?
>>
>>
>

Major CI changes PR

[Sean Goller]

Hi, I've submitted a PR  to make
the pipeline much more fork friendly for ease in testing. As soon as this
PR gets merged, I expect some degree of instability and redeployment of
pipelines, so please be aware of that if you see issues.

-Sean.

Re: Proposed API for a ConcurrencyTestHelper

[Dan Smith]

Hi Helena,

Looks great! I think this will make writing multithreaded tests a lot less
error prone. A couple of minor comments below:

I'm not sure we need runInSeries, someone could do that without using this
test helper.

Could we make the default timeout something more like 5 minutes? See this
thread https://markmail.org/thread/yrcvdlbixqojl6oh


-Dan

On Wed, Jul 18, 2018 at 12:16 PM, Dale Emery  wrote:

> Hi Helena,
>
> I like it!
>
> A few suggestions:
>
> - For the durations (repeatForDuration() and setTimeout()), consider using
> a java.time.Duration rather than an int to express the duration. The
> Duration class lets most of the code not have to keep track of the time
> units. The caller creates the Duration using whatever time unit they want.
> When some other code needs to convert it to a specific time unit, there are
> methods for that. The rest of the code doesn’t have to care about the time
> units. And there are methods for adding, subtracting, and comparing
> Durations and Instants, all handling the time units nicely.
>
> - Consider adding a version of runAndExpectValue that takes a Predicate
> instead of a value. And maybe one taking a Hamcrest Matcher.
>
> - Consider specifying the type of the Callable, and preferably for the
> value passed to runAndExpectValue()). And definitely for Predicates and
> Matchers, if you add those methods.
>
> Cheers,
> Dale
>
> > On Jul 18, 2018, at 11:53 AM, Helena Bales  wrote:
> >
> > Hello All,
> >
> > Below I have included a proposed API for a concurrency test helper. The
> > goal of this test helper is to make it easier for tests to use multiple
> > threads. I believe providing a helper for this will reduce the number of
> > tests that incorrectly use threads, and help make our DUnit tests more
> > readable.
> >
> > I would like feedback on the API below. I have provided the method
> > signatures that I plan on implementing, as well as an example usage.
> Please
> > let me know if there are any methods that seem unnecessary or if there
> are
> > any important use cases that I missed here.
> >
> > Thank you,
> > Helena Bales
> >
> > public class ConcurrencyTestHelper {
> >  private List> toRun;
> >  private Integer timeout;
> >
> >  private final Integer DEFAULT_TIMEOUT = 30;
> >
> >  /**
> >   * A default constructor that sets the timeout to a default of 30
> seconds
> >   */
> >  public ConcurrencyTestHelper() {
> >toRun = new ArrayList>();
> >timeout = DEFAULT_TIMEOUT;
> >  }
> >
> >  /**
> >   * Run the given Runnable and assert that an exception of the given
> > type is thrown. The thread
> >   * will be cancelled when the class timeout is reached.
> >   * @param runnable a Runnable that throws an exception and responds
> > to cancellation signal
> >   * @param exceptionType a Class of Exception that is expected to be
> > thrown by the runnable
> >   */
> >  public void runAndExpectException(Runnable runnable, Class
> exceptionType);
> >
> >  /**
> >   * Run the given Runnable and assert that no exceptions are thrown.
> > The thread will be cancelled
> >   * when the class timeout is reached.
> >   * @param runnable a Runnable that does not throw an exception and
> > responds to cancellation signal
> >   */
> >  public void runAndExpectNoException(Runnable runnable);
> >
> >  /**
> >   * Run the given Callable and assert that the result of that
> > callable is the given expected value.
> >   * The thread will be cancelled when the class timeout is reached.
> >   * @param callable a Callable that returns an object, does not throw
> > exceptions and responds to
> >   * cancellation signal
> >   * @param value an Object that is the expected return value from the
> Callable.
> >   */
> >  public void runAndExpectValue(Callable callable, Object value);
> >
> >  /**
> >   * Run the given runnable in a loop until the rest of the threads
> > complete. Fails if the runnable
> >   * throws an exception during any iteration. After the rest of the
> > threads complete the loop will
> >   * not restart the runnable. The running iteration will be cancelled
> > when the timeout set on the
> >   * class is reached. A cancellation will result in a test failure.
> >   * @param runnable a Runnable that does not throw an exception and
> > responds to cancellation signal
> >   */
> >  public void repeatUntilAllThreadsComplete(Runnable runnable);
> >
> >  /**
> >   * Run the given runnable in a loop for some number of iterations.
> > Fails if the runnable throws an
> >   * exception in any iteration. After the number of iterations has
> > been met, the loop will not
> >   * restart the runnable. The running iteration of the Runnable will
> > be cancelled when the
> >   * timeout set on the class is reached, regardless of if the number
> > of desired iterations has
> >   * been reached. A cancellation will result in a test failure.
> >   * @param runnable a Runnable that does not throw an exception and
> > responds to cancellation signal
> >   * @param iteration

Re: How to setup IntelliJ

[Jacob Barrett]

The fix has been merged. It only fixes the compilation from IJ. The editor
will continue to colorize dependencies from other test sources as missing.
This can only be fixed by completing the extraction of the test framework
sources into their own module. IJ editor assumes that test code should
never depend on test code, which is a correct assumption but causes issues
for us. We need to extract framework code into its own module as main
source so that we can fix our gradle dependencies and correct this IJ issue.

Are there any takers interested in extracting DUnit and other framework
sources into their own modules?




On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett  wrote:

> All,
>
> I have a fix that appears to address all these issue in a PR. I am just
> waiting for them to pass come CI because they effect the Gradle build too.
> Please don’t check in any “fixes” to this IJ integration issue.
>
> -Jake
>
>
> > On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> >
> > More details: If I open up BasicDistributedTest, I can run it but all of
> > the imports for classes that are also in distributedTest are RED and
> don't
> > show up in the Project window.
> >
> > After pulling, the following commit is now my head revision.
> Unfortunately,
> > it doesn't fix my problem. But it did cause one change: If I search
> > for BasicDistributedTest,
> > it now finds two instances -- one is found in geode-core_distributedTest
> > and the other is found in geode-core_test.
> >
> > commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
> > *origin/develop*)
> >
> > Author: Jinmei Liao 
> >
> > Date:   Wed Jul 18 10:22:16 2018 -0700
> >
> >
> >GEODE-5363: Fixes issue with build in IJ IDEA.
> >
> >
> >> On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund  wrote:
> >>
> >> Yes, my head revision is...
> >>
> >> commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
> >> *origin/develop*, *origin/HEAD*)
> >>
> >> Author: Anthony Baker 
> >>
> >> Date:   Mon Jul 16 16:56:03 2018 -0700
> >>
> >>
> >>GEODE-5421 Updated dependencies
> >>
> >>
> >>
> >>Updated bundled library dependencies. The updated libaries are:
> >>
> >>
> >>
> >>HikariCP 3.0.0 -> 3.2.0
> >>
> >>fast-classpath-scanner 2.19.0 -> 2.21
> >>
> >>fastutil 8.1.1 -> 8.2.1
> >>
> >>google-gson 2.8.2 -> 2.8.5
> >>
> >>guava 24.1-jre -> 25.1-jre
> >>
> >>jackson 2.9.5 -> 2.9.6
> >>
> >>jansi 1.17 -> 1.17.1
> >>
> >>netty 4.1.21.Final -> 4.1.27.Final
> >>
> >>protobuf 3.5.1 -> 3.6.0
> >>
> >>spring-security 4.2.4 -> 4.2.7
> >>
> >>spring-* 4.3.14 -> 4.3.18
> >>
> >>springfox 2.8.0 -> 2.9.2
> >>
> >>
> >>
> >>These changes mean that javassist and reflections are no longer
> >>
> >>bundled with the binary convenience distributions.
> >>
> >>
> >>
> >>
> >>> On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker 
> wrote:
> >>>
> >>> Are you using this commit?
> >>>
> >>> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
> >>> Author: Jacob Barrett 
> >>> Date:   Tue Jul 17 17:03:22 2018 -0700
> >>>
> >>>GEODE-5363: Fixes issue with build in IJ IDEA.
> >>>
> >>>- Splits up LuceneTestUtilities to removed duplication and compile
> >>> fail.
> >>>
> >>> Anthony
> >>>
> >>>
>  On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
> 
>  Does anyone have any instructions on setting up IntelliJ with the
> latest
>  changes? I’ve tried new project from sources and I’ve tried using
> >>> ./gradlew
>  idea — neither works for me. Basically all non-unit tests end up not
> >>> being
>  a source set.
> >>>
> >>>
> >>
>

Re: Proposed API for a ConcurrencyTestHelper

[Jacob Barrett]

+1

Echo Dales suggestion for Duration over int for time.

-Jake


On Wed, Jul 18, 2018 at 12:58 PM Dan Smith  wrote:

> Hi Helena,
>
> Looks great! I think this will make writing multithreaded tests a lot less
> error prone. A couple of minor comments below:
>
> I'm not sure we need runInSeries, someone could do that without using this
> test helper.
>
> Could we make the default timeout something more like 5 minutes? See this
> thread https://markmail.org/thread/yrcvdlbixqojl6oh
>
>
> -Dan
>
> On Wed, Jul 18, 2018 at 12:16 PM, Dale Emery  wrote:
>
> > Hi Helena,
> >
> > I like it!
> >
> > A few suggestions:
> >
> > - For the durations (repeatForDuration() and setTimeout()), consider
> using
> > a java.time.Duration rather than an int to express the duration. The
> > Duration class lets most of the code not have to keep track of the time
> > units. The caller creates the Duration using whatever time unit they
> want.
> > When some other code needs to convert it to a specific time unit, there
> are
> > methods for that. The rest of the code doesn’t have to care about the
> time
> > units. And there are methods for adding, subtracting, and comparing
> > Durations and Instants, all handling the time units nicely.
> >
> > - Consider adding a version of runAndExpectValue that takes a Predicate
> > instead of a value. And maybe one taking a Hamcrest Matcher.
> >
> > - Consider specifying the type of the Callable, and preferably for the
> > value passed to runAndExpectValue()). And definitely for Predicates and
> > Matchers, if you add those methods.
> >
> > Cheers,
> > Dale
> >
> > > On Jul 18, 2018, at 11:53 AM, Helena Bales  wrote:
> > >
> > > Hello All,
> > >
> > > Below I have included a proposed API for a concurrency test helper. The
> > > goal of this test helper is to make it easier for tests to use multiple
> > > threads. I believe providing a helper for this will reduce the number
> of
> > > tests that incorrectly use threads, and help make our DUnit tests more
> > > readable.
> > >
> > > I would like feedback on the API below. I have provided the method
> > > signatures that I plan on implementing, as well as an example usage.
> > Please
> > > let me know if there are any methods that seem unnecessary or if there
> > are
> > > any important use cases that I missed here.
> > >
> > > Thank you,
> > > Helena Bales
> > >
> > > public class ConcurrencyTestHelper {
> > >  private List> toRun;
> > >  private Integer timeout;
> > >
> > >  private final Integer DEFAULT_TIMEOUT = 30;
> > >
> > >  /**
> > >   * A default constructor that sets the timeout to a default of 30
> > seconds
> > >   */
> > >  public ConcurrencyTestHelper() {
> > >toRun = new ArrayList>();
> > >timeout = DEFAULT_TIMEOUT;
> > >  }
> > >
> > >  /**
> > >   * Run the given Runnable and assert that an exception of the given
> > > type is thrown. The thread
> > >   * will be cancelled when the class timeout is reached.
> > >   * @param runnable a Runnable that throws an exception and responds
> > > to cancellation signal
> > >   * @param exceptionType a Class of Exception that is expected to be
> > > thrown by the runnable
> > >   */
> > >  public void runAndExpectException(Runnable runnable, Class
> > exceptionType);
> > >
> > >  /**
> > >   * Run the given Runnable and assert that no exceptions are thrown.
> > > The thread will be cancelled
> > >   * when the class timeout is reached.
> > >   * @param runnable a Runnable that does not throw an exception and
> > > responds to cancellation signal
> > >   */
> > >  public void runAndExpectNoException(Runnable runnable);
> > >
> > >  /**
> > >   * Run the given Callable and assert that the result of that
> > > callable is the given expected value.
> > >   * The thread will be cancelled when the class timeout is reached.
> > >   * @param callable a Callable that returns an object, does not throw
> > > exceptions and responds to
> > >   * cancellation signal
> > >   * @param value an Object that is the expected return value from the
> > Callable.
> > >   */
> > >  public void runAndExpectValue(Callable callable, Object value);
> > >
> > >  /**
> > >   * Run the given runnable in a loop until the rest of the threads
> > > complete. Fails if the runnable
> > >   * throws an exception during any iteration. After the rest of the
> > > threads complete the loop will
> > >   * not restart the runnable. The running iteration will be cancelled
> > > when the timeout set on the
> > >   * class is reached. A cancellation will result in a test failure.
> > >   * @param runnable a Runnable that does not throw an exception and
> > > responds to cancellation signal
> > >   */
> > >  public void repeatUntilAllThreadsComplete(Runnable runnable);
> > >
> > >  /**
> > >   * Run the given runnable in a loop for some number of iterations.
> > > Fails if the runnable throws an
> > >   * exception in any iteration. After the number of iterations has
> > > been met, the loop will not
> > >   * restart the runn

Re: Proposed API for a ConcurrencyTestHelper

[Kirk Lund]

I've actually been trying to remove most of the Util and Helper classes
used by tests. My reasons are two-fold: 1) these classes tend to be very
non-OOP, 2) they tend to introduce a new API to the test which then
obscures what it's actually doing. When I look at a test, I don't want
anything hidden -- I want to SEE everything that the test is doing and
using, and I especially want to see the Geode user APIs that the test uses
(other Helper classes have hidden the user APIs). At the very least, please
try to steer away from using Util and Helper in a class name.

I would recommend a couple more changes. First off, decide what this object
is. For example, is it an Executor or ExecutorService? Should it be a JUnit
Rule instead of Util/Helper? Next simplify the API to match one of those.
See our ExecutorServiceRule which is in geode-junit. Is that similar to
what you're trying to create? Maybe we need just to
modify ExecutorServiceRule instead of introduce yet another Helper class. I
don't see why we need to add another class that is so similar to both
ExecutorServiceRule
and AsyncInvocation -- I vote to evolve those existing classes instead of
adding something new.

On Wed, Jul 18, 2018 at 11:53 AM, Helena Bales  wrote:

> Hello All,
>
> Below I have included a proposed API for a concurrency test helper. The
> goal of this test helper is to make it easier for tests to use multiple
> threads. I believe providing a helper for this will reduce the number of
> tests that incorrectly use threads, and help make our DUnit tests more
> readable.
>
> I would like feedback on the API below. I have provided the method
> signatures that I plan on implementing, as well as an example usage. Please
> let me know if there are any methods that seem unnecessary or if there are
> any important use cases that I missed here.
>
> Thank you,
> Helena Bales
>
> public class ConcurrencyTestHelper {
>   private List> toRun;
>   private Integer timeout;
>
>   private final Integer DEFAULT_TIMEOUT = 30;
>
>   /**
>* A default constructor that sets the timeout to a default of 30 seconds
>*/
>   public ConcurrencyTestHelper() {
> toRun = new ArrayList>();
> timeout = DEFAULT_TIMEOUT;
>   }
>
>   /**
>* Run the given Runnable and assert that an exception of the given
> type is thrown. The thread
>* will be cancelled when the class timeout is reached.
>* @param runnable a Runnable that throws an exception and responds
> to cancellation signal
>* @param exceptionType a Class of Exception that is expected to be
> thrown by the runnable
>*/
>   public void runAndExpectException(Runnable runnable, Class
> exceptionType);
>
>   /**
>* Run the given Runnable and assert that no exceptions are thrown.
> The thread will be cancelled
>* when the class timeout is reached.
>* @param runnable a Runnable that does not throw an exception and
> responds to cancellation signal
>*/
>   public void runAndExpectNoException(Runnable runnable);
>
>   /**
>* Run the given Callable and assert that the result of that
> callable is the given expected value.
>* The thread will be cancelled when the class timeout is reached.
>* @param callable a Callable that returns an object, does not throw
> exceptions and responds to
>* cancellation signal
>* @param value an Object that is the expected return value from the
> Callable.
>*/
>   public void runAndExpectValue(Callable callable, Object value);
>
>   /**
>* Run the given runnable in a loop until the rest of the threads
> complete. Fails if the runnable
>* throws an exception during any iteration. After the rest of the
> threads complete the loop will
>* not restart the runnable. The running iteration will be cancelled
> when the timeout set on the
>* class is reached. A cancellation will result in a test failure.
>* @param runnable a Runnable that does not throw an exception and
> responds to cancellation signal
>*/
>   public void repeatUntilAllThreadsComplete(Runnable runnable);
>
>   /**
>* Run the given runnable in a loop for some number of iterations.
> Fails if the runnable throws an
>* exception in any iteration. After the number of iterations has
> been met, the loop will not
>* restart the runnable. The running iteration of the Runnable will
> be cancelled when the
>* timeout set on the class is reached, regardless of if the number
> of desired iterations has
>* been reached. A cancellation will result in a test failure.
>* @param runnable a Runnable that does not throw an exception and
> responds to cancellation signal
>* @param iterations a maximum number of iterations to repeat for
>*/
>   public void repeatForIterations(Runnable runnable, Integer iterations);
>
>   /**
>* Run the given runnable in a loop for a certain duration. Fails if
> the runnable throws an
>* exception in any iteration. After the duration has been exceeded,
> the loop will not restart
>* the r

Re: Moving from gradle to maven

[Jens Deppe]

+1 For moving to maven.

I think the blog Kirk linked hits all the relevant pain points.

This is the third time we've done significant Gradle work and every time it
is painful. It's also probably never going to get any better.

For myself, Gradle certainly feels like a lot of magic happening under the
covers - it feels like it requires a fair bit of mental effort to
understand and distinguish configuration phases and execution phases and
which parts fit into which phase. Maven has its own magic, but is
definitely more linear and obvious in it's execution steps.

--Jens

On Wed, Jul 18, 2018 at 12:27 PM Patrick Rhomberg 
wrote:

> +1 to correcting our current broken gradle build.
>
>
> The fault, dear Brutus, is not in our [tools], but in ourselves.
>
> I think the root pain point is that our dependency tree is neither explicit
> nor correct in several places.  I have myself had frequent issues
> surrounding our Protobuf and OQLLexer classes requiring a command-line
> build and re-import.  It's also why, after we bumped gradle versions, we
> were prone to errors when building in parallel.
>
> Correctly documenting and making explicit the gradle build dependencies is
> something that I am meaning to look into soon, but I am currently looking
> into improving our pipelines and metrics scripting.
>
> On Wed, Jul 18, 2018 at 12:04 PM, Udo Kohlmeyer  wrote:
>
> > I must agree, the fact that IntelliJ cannot handle the current project
> > structure, is that I believe that we have a complicated project
> structure.
> > Moving to maven would force a more strict project structure.
> >
> > I don't mind moving to maven, but I believe that we would have similar
> > experiences with maven and a complex project structure. I was thinking
> > would could move to Gradle-Kotlin DSL, but that also would not solve the
> > current structure problem.
> >
> > So...  +1 on move to maven OR +1 on refactoring to the current gradle
> > setup to be less "custom" and maybe a little more rigid.
> >
> >
> > On 7/18/18 11:00, Kirk Lund wrote:
> >
> >> Gradle appears to not play well with IntelliJ unless the project is
> overly
> >> simple. I don't want to spend my days fighting with tools that don't
> work
> >> well together.
> >>
> >> Here's an interesting blog article about moving from gradle to maven:
> >>
> >> https://blog.philipphauer.de/moving-back-from-gradle-to-maven/
> >>
> >> Any other data points or opinions about moving from gradle to maven?
> >>
> >>
> >
>

Re: How to setup IntelliJ

[Kirk Lund]

Sai had attempted to extract our testing framework(s) including DUnit to a
new geode-test module. I thought it had been merged to develop but it seems
to have been reverted. Anyone know why it had to be reverted?

Other than moving to maven, another option would be to separate test types
based on file name: *IntegrationTest, *DistributedTest, *Test
(excluding *IntegrationTest,
*DistributedTest). It's perhaps not as elegant as separate srcSets but at
least Gradle and the IJ plugin can handle it properly.

On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett  wrote:

> The fix has been merged. It only fixes the compilation from IJ. The editor
> will continue to colorize dependencies from other test sources as missing.
> This can only be fixed by completing the extraction of the test framework
> sources into their own module. IJ editor assumes that test code should
> never depend on test code, which is a correct assumption but causes issues
> for us. We need to extract framework code into its own module as main
> source so that we can fix our gradle dependencies and correct this IJ
> issue.
>
> Are there any takers interested in extracting DUnit and other framework
> sources into their own modules?
>
>
>
>
> On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett 
> wrote:
>
> > All,
> >
> > I have a fix that appears to address all these issue in a PR. I am just
> > waiting for them to pass come CI because they effect the Gradle build
> too.
> > Please don’t check in any “fixes” to this IJ integration issue.
> >
> > -Jake
> >
> >
> > > On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> > >
> > > More details: If I open up BasicDistributedTest, I can run it but all
> of
> > > the imports for classes that are also in distributedTest are RED and
> > don't
> > > show up in the Project window.
> > >
> > > After pulling, the following commit is now my head revision.
> > Unfortunately,
> > > it doesn't fix my problem. But it did cause one change: If I search
> > > for BasicDistributedTest,
> > > it now finds two instances -- one is found in
> geode-core_distributedTest
> > > and the other is found in geode-core_test.
> > >
> > > commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
> > > *origin/develop*)
> > >
> > > Author: Jinmei Liao 
> > >
> > > Date:   Wed Jul 18 10:22:16 2018 -0700
> > >
> > >
> > >GEODE-5363: Fixes issue with build in IJ IDEA.
> > >
> > >
> > >> On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund  wrote:
> > >>
> > >> Yes, my head revision is...
> > >>
> > >> commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
> > >> *origin/develop*, *origin/HEAD*)
> > >>
> > >> Author: Anthony Baker 
> > >>
> > >> Date:   Mon Jul 16 16:56:03 2018 -0700
> > >>
> > >>
> > >>GEODE-5421 Updated dependencies
> > >>
> > >>
> > >>
> > >>Updated bundled library dependencies. The updated libaries are:
> > >>
> > >>
> > >>
> > >>HikariCP 3.0.0 -> 3.2.0
> > >>
> > >>fast-classpath-scanner 2.19.0 -> 2.21
> > >>
> > >>fastutil 8.1.1 -> 8.2.1
> > >>
> > >>google-gson 2.8.2 -> 2.8.5
> > >>
> > >>guava 24.1-jre -> 25.1-jre
> > >>
> > >>jackson 2.9.5 -> 2.9.6
> > >>
> > >>jansi 1.17 -> 1.17.1
> > >>
> > >>netty 4.1.21.Final -> 4.1.27.Final
> > >>
> > >>protobuf 3.5.1 -> 3.6.0
> > >>
> > >>spring-security 4.2.4 -> 4.2.7
> > >>
> > >>spring-* 4.3.14 -> 4.3.18
> > >>
> > >>springfox 2.8.0 -> 2.9.2
> > >>
> > >>
> > >>
> > >>These changes mean that javassist and reflections are no longer
> > >>
> > >>bundled with the binary convenience distributions.
> > >>
> > >>
> > >>
> > >>
> > >>> On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker 
> > wrote:
> > >>>
> > >>> Are you using this commit?
> > >>>
> > >>> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
> > >>> Author: Jacob Barrett 
> > >>> Date:   Tue Jul 17 17:03:22 2018 -0700
> > >>>
> > >>>GEODE-5363: Fixes issue with build in IJ IDEA.
> > >>>
> > >>>- Splits up LuceneTestUtilities to removed duplication and compile
> > >>> fail.
> > >>>
> > >>> Anthony
> > >>>
> > >>>
> >  On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
> > 
> >  Does anyone have any instructions on setting up IntelliJ with the
> > latest
> >  changes? I’ve tried new project from sources and I’ve tried using
> > >>> ./gradlew
> >  idea — neither works for me. Basically all non-unit tests end up not
> > >>> being
> >  a source set.
> > >>>
> > >>>
> > >>
> >
>

Re: Moving from gradle to maven

[Sean Goller]

This is a non starter without a maven equivalent of the gradle dockerized
plugin. Switching to maven without that will mean longer testing times,
which I feel is unacceptable.

So far I've found this reference on stack overflow (
https://stackoverflow.com/questions/36808351/running-junit-tests-in-parallel-with-docker
) to a homebuilt solution, but I'm unsure how replicable it is.

-Sean.

On Wed, Jul 18, 2018 at 1:21 PM Jens Deppe  wrote:

> +1 For moving to maven.
>
> I think the blog Kirk linked hits all the relevant pain points.
>
> This is the third time we've done significant Gradle work and every time it
> is painful. It's also probably never going to get any better.
>
> For myself, Gradle certainly feels like a lot of magic happening under the
> covers - it feels like it requires a fair bit of mental effort to
> understand and distinguish configuration phases and execution phases and
> which parts fit into which phase. Maven has its own magic, but is
> definitely more linear and obvious in it's execution steps.
>
> --Jens
>
> On Wed, Jul 18, 2018 at 12:27 PM Patrick Rhomberg 
> wrote:
>
> > +1 to correcting our current broken gradle build.
> >
> >
> > The fault, dear Brutus, is not in our [tools], but in ourselves.
> >
> > I think the root pain point is that our dependency tree is neither
> explicit
> > nor correct in several places.  I have myself had frequent issues
> > surrounding our Protobuf and OQLLexer classes requiring a command-line
> > build and re-import.  It's also why, after we bumped gradle versions, we
> > were prone to errors when building in parallel.
> >
> > Correctly documenting and making explicit the gradle build dependencies
> is
> > something that I am meaning to look into soon, but I am currently looking
> > into improving our pipelines and metrics scripting.
> >
> > On Wed, Jul 18, 2018 at 12:04 PM, Udo Kohlmeyer  wrote:
> >
> > > I must agree, the fact that IntelliJ cannot handle the current project
> > > structure, is that I believe that we have a complicated project
> > structure.
> > > Moving to maven would force a more strict project structure.
> > >
> > > I don't mind moving to maven, but I believe that we would have similar
> > > experiences with maven and a complex project structure. I was thinking
> > > would could move to Gradle-Kotlin DSL, but that also would not solve
> the
> > > current structure problem.
> > >
> > > So...  +1 on move to maven OR +1 on refactoring to the current gradle
> > > setup to be less "custom" and maybe a little more rigid.
> > >
> > >
> > > On 7/18/18 11:00, Kirk Lund wrote:
> > >
> > >> Gradle appears to not play well with IntelliJ unless the project is
> > overly
> > >> simple. I don't want to spend my days fighting with tools that don't
> > work
> > >> well together.
> > >>
> > >> Here's an interesting blog article about moving from gradle to maven:
> > >>
> > >> https://blog.philipphauer.de/moving-back-from-gradle-to-maven/
> > >>
> > >> Any other data points or opinions about moving from gradle to maven?
> > >>
> > >>
> > >
> >
>

Re: Proposed API for a ConcurrencyTestHelper

[Dan Smith]

> See our ExecutorServiceRule which is in geode-junit. Is that similar to
> what you're trying to create? Maybe we need just to
> modify ExecutorServiceRule instead of introduce yet another Helper class. I
> don't see why we need to add another class that is so similar to both
> ExecutorServiceRule
> and AsyncInvocation -- I vote to evolve those existing classes instead of
> adding something new.
>

I see this as potentially wrapping that ExecutorServiceRule (which, btw, I
had no idea existed. Nice!). That rule lets you fire off asynchronous
tasks. But generally a test doesn't want to just fire off tasks; it needs
to get the results back and see what happened. For example, if you look at
GMSEncryptJUnitTest which uses ExecutorServiceRule. It has to use a
CountDownLatch to make sure the threads finish, and in fact it never checks
to see if any of the threads throw an exception! With this helper that test
could be more readable and would actually fail if it hit an error.

Maybe this could be an another rule that embeds and ExecutorServiceRule, or
just have a constructor that takes an executor service, to work with that
existing rule?

BTW, why does ExecutorServiceRule default to 1 thread? Unlimited threads
seems like a more sensible default for tests that want an executor.

-Dan

Re: Moving from gradle to maven

[Jens Deppe]

Right regarding the dockerized plugin - that was another pain point when we
moved to from 3.x to 4.x. We'll run into the same thing again with every
Gradle update when there is an API change.

If we think it's worth moving to maven we should do a spike on producing a
dockerized test runner.

--Jens

On Wed, Jul 18, 2018 at 1:34 PM Sean Goller  wrote:

> This is a non starter without a maven equivalent of the gradle dockerized
> plugin. Switching to maven without that will mean longer testing times,
> which I feel is unacceptable.
>
> So far I've found this reference on stack overflow (
>
> https://stackoverflow.com/questions/36808351/running-junit-tests-in-parallel-with-docker
> ) to a homebuilt solution, but I'm unsure how replicable it is.
>
> -Sean.
>
> On Wed, Jul 18, 2018 at 1:21 PM Jens Deppe  wrote:
>
> > +1 For moving to maven.
> >
> > I think the blog Kirk linked hits all the relevant pain points.
> >
> > This is the third time we've done significant Gradle work and every time
> it
> > is painful. It's also probably never going to get any better.
> >
> > For myself, Gradle certainly feels like a lot of magic happening under
> the
> > covers - it feels like it requires a fair bit of mental effort to
> > understand and distinguish configuration phases and execution phases and
> > which parts fit into which phase. Maven has its own magic, but is
> > definitely more linear and obvious in it's execution steps.
> >
> > --Jens
> >
> > On Wed, Jul 18, 2018 at 12:27 PM Patrick Rhomberg 
> > wrote:
> >
> > > +1 to correcting our current broken gradle build.
> > >
> > >
> > > The fault, dear Brutus, is not in our [tools], but in ourselves.
> > >
> > > I think the root pain point is that our dependency tree is neither
> > explicit
> > > nor correct in several places.  I have myself had frequent issues
> > > surrounding our Protobuf and OQLLexer classes requiring a command-line
> > > build and re-import.  It's also why, after we bumped gradle versions,
> we
> > > were prone to errors when building in parallel.
> > >
> > > Correctly documenting and making explicit the gradle build dependencies
> > is
> > > something that I am meaning to look into soon, but I am currently
> looking
> > > into improving our pipelines and metrics scripting.
> > >
> > > On Wed, Jul 18, 2018 at 12:04 PM, Udo Kohlmeyer 
> wrote:
> > >
> > > > I must agree, the fact that IntelliJ cannot handle the current
> project
> > > > structure, is that I believe that we have a complicated project
> > > structure.
> > > > Moving to maven would force a more strict project structure.
> > > >
> > > > I don't mind moving to maven, but I believe that we would have
> similar
> > > > experiences with maven and a complex project structure. I was
> thinking
> > > > would could move to Gradle-Kotlin DSL, but that also would not solve
> > the
> > > > current structure problem.
> > > >
> > > > So...  +1 on move to maven OR +1 on refactoring to the current gradle
> > > > setup to be less "custom" and maybe a little more rigid.
> > > >
> > > >
> > > > On 7/18/18 11:00, Kirk Lund wrote:
> > > >
> > > >> Gradle appears to not play well with IntelliJ unless the project is
> > > overly
> > > >> simple. I don't want to spend my days fighting with tools that don't
> > > work
> > > >> well together.
> > > >>
> > > >> Here's an interesting blog article about moving from gradle to
> maven:
> > > >>
> > > >> https://blog.philipphauer.de/moving-back-from-gradle-to-maven/
> > > >>
> > > >> Any other data points or opinions about moving from gradle to maven?
> > > >>
> > > >>
> > > >
> > >
> >
>

Re: How to setup IntelliJ

[Jacob Barrett]

Even maven won’t fix the issue remain in IJ. We need to refactor the code so 
that Test code does not depend on Test code. It’s just bad form to do so.

-Jake


> On Jul 18, 2018, at 1:28 PM, Kirk Lund  wrote:
> 
> Sai had attempted to extract our testing framework(s) including DUnit to a
> new geode-test module. I thought it had been merged to develop but it seems
> to have been reverted. Anyone know why it had to be reverted?
> 
> Other than moving to maven, another option would be to separate test types
> based on file name: *IntegrationTest, *DistributedTest, *Test
> (excluding *IntegrationTest,
> *DistributedTest). It's perhaps not as elegant as separate srcSets but at
> least Gradle and the IJ plugin can handle it properly.
> 
>> On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett  wrote:
>> 
>> The fix has been merged. It only fixes the compilation from IJ. The editor
>> will continue to colorize dependencies from other test sources as missing.
>> This can only be fixed by completing the extraction of the test framework
>> sources into their own module. IJ editor assumes that test code should
>> never depend on test code, which is a correct assumption but causes issues
>> for us. We need to extract framework code into its own module as main
>> source so that we can fix our gradle dependencies and correct this IJ
>> issue.
>> 
>> Are there any takers interested in extracting DUnit and other framework
>> sources into their own modules?
>> 
>> 
>> 
>> 
>> On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett 
>> wrote:
>> 
>>> All,
>>> 
>>> I have a fix that appears to address all these issue in a PR. I am just
>>> waiting for them to pass come CI because they effect the Gradle build
>> too.
>>> Please don’t check in any “fixes” to this IJ integration issue.
>>> 
>>> -Jake
>>> 
>>> 
 On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
 
 More details: If I open up BasicDistributedTest, I can run it but all
>> of
 the imports for classes that are also in distributedTest are RED and
>>> don't
 show up in the Project window.
 
 After pulling, the following commit is now my head revision.
>>> Unfortunately,
 it doesn't fix my problem. But it did cause one change: If I search
 for BasicDistributedTest,
 it now finds two instances -- one is found in
>> geode-core_distributedTest
 and the other is found in geode-core_test.
 
 commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
 *origin/develop*)
 
 Author: Jinmei Liao 
 
 Date:   Wed Jul 18 10:22:16 2018 -0700
 
 
   GEODE-5363: Fixes issue with build in IJ IDEA.
 
 
> On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund  wrote:
> 
> Yes, my head revision is...
> 
> commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
> *origin/develop*, *origin/HEAD*)
> 
> Author: Anthony Baker 
> 
> Date:   Mon Jul 16 16:56:03 2018 -0700
> 
> 
>   GEODE-5421 Updated dependencies
> 
> 
> 
>   Updated bundled library dependencies. The updated libaries are:
> 
> 
> 
>   HikariCP 3.0.0 -> 3.2.0
> 
>   fast-classpath-scanner 2.19.0 -> 2.21
> 
>   fastutil 8.1.1 -> 8.2.1
> 
>   google-gson 2.8.2 -> 2.8.5
> 
>   guava 24.1-jre -> 25.1-jre
> 
>   jackson 2.9.5 -> 2.9.6
> 
>   jansi 1.17 -> 1.17.1
> 
>   netty 4.1.21.Final -> 4.1.27.Final
> 
>   protobuf 3.5.1 -> 3.6.0
> 
>   spring-security 4.2.4 -> 4.2.7
> 
>   spring-* 4.3.14 -> 4.3.18
> 
>   springfox 2.8.0 -> 2.9.2
> 
> 
> 
>   These changes mean that javassist and reflections are no longer
> 
>   bundled with the binary convenience distributions.
> 
> 
> 
> 
>> On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker 
>>> wrote:
>> 
>> Are you using this commit?
>> 
>> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
>> Author: Jacob Barrett 
>> Date:   Tue Jul 17 17:03:22 2018 -0700
>> 
>>   GEODE-5363: Fixes issue with build in IJ IDEA.
>> 
>>   - Splits up LuceneTestUtilities to removed duplication and compile
>> fail.
>> 
>> Anthony
>> 
>> 
>>> On Jul 18, 2018, at 9:55 AM, Kirk Lund  wrote:
>>> 
>>> Does anyone have any instructions on setting up IntelliJ with the
>>> latest
>>> changes? I’ve tried new project from sources and I’ve tried using
>> ./gradlew
>>> idea — neither works for me. Basically all non-unit tests end up not
>> being
>>> a source set.
>> 
>> 
> 
>>> 
>> 

Geode unit tests completed in 'develop/UITests' with non-zero exit code

[apachegeodeci]

Pipeline results can be found at:

Concourse: 
https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/UITests/builds/176

Re: Proposed API for a ConcurrencyTestHelper

[Kirk Lund]

I'd love to collaborate on improving ExecutorServiceRule. Let me know if
you or anyone else is interested.

Honestly, I can't remember why I had it default to one thread unless I was
favoring very explicit and tight control over what resources the tests use.
Unlimited sounds reasonable for this.

GMSEncryptJUnitTest isn't a good example (for ExecutorServiceRule or for
testing). When I introduced ExecutorServiceRule, I expected most use cases
to involve Future which does handle exceptions, or a developer might use
ErrorCollector (a JUnit rule). I didn't fix all the issues in
GMSEncryptJUnitTest
-- I simply refactored any tests that were using ExecutorService to use the
rule when I introduced ExecutorServiceRule. The test is no worse than
before changing it to use the rule but yes, I see now that the test could
use some further cleanup.

See the use of Future within
RegionVersionVectorTest.doesNotHangIfOtherThreadChangedVersion
(this is the test for which I created rule):

Future result = executorServiceRule.runAsync(() ->
rvv.updateLocalVersion(newVersion));
...
assertThatCode(() -> result.get(2, SECONDS)).doesNotThrowAnyException();

Other than using Futures with the rule, I would have expected a developer
to use ErrorCollector in an IntegrationTest or SharedErrorCollector in a
DistributedTest. Ultimately, I imagined that the use of Futures would be
the primary usage pattern for ExecutorServiceRule and Future does handle
and rethrow (wrapped within ExecutionException) any exceptions thrown by
the code being executed.

ExecutorServiceRule, ErrorCollector/SharedErrorCollector and
AsyncInvocation (which is a Future) could maybe be combined in some
interesting ways.

On Wed, Jul 18, 2018 at 1:49 PM, Dan Smith  wrote:

> > See our ExecutorServiceRule which is in geode-junit. Is that similar to
> > what you're trying to create? Maybe we need just to
> > modify ExecutorServiceRule instead of introduce yet another Helper
> class. I
> > don't see why we need to add another class that is so similar to both
> > ExecutorServiceRule
> > and AsyncInvocation -- I vote to evolve those existing classes instead of
> > adding something new.
> >
>
> I see this as potentially wrapping that ExecutorServiceRule (which, btw, I
> had no idea existed. Nice!). That rule lets you fire off asynchronous
> tasks. But generally a test doesn't want to just fire off tasks; it needs
> to get the results back and see what happened. For example, if you look at
> GMSEncryptJUnitTest which uses ExecutorServiceRule. It has to use a
> CountDownLatch to make sure the threads finish, and in fact it never checks
> to see if any of the threads throw an exception! With this helper that test
> could be more readable and would actually fail if it hit an error.
>
> Maybe this could be an another rule that embeds and ExecutorServiceRule, or
> just have a constructor that takes an executor service, to work with that
> existing rule?
>
> BTW, why does ExecutorServiceRule default to 1 thread? Unlimited threads
> seems like a more sensible default for tests that want an executor.
>
> -Dan
>

Re: How to setup IntelliJ

[Jinmei Liao]

With Jake's merge, the my integration/distributed tests in geode-cq (and
maybe all other non-core modules) are not compiling nicely in IDEA. If you
want a quick fix to your IDEA issue and not waiting till all the test
refactors are done, you can add these lines in the beginning of that
modules build.gradle's dependency section:

integrationTestCompile files(project(':geode-core').buildDir.absolutePath+
"/../out/test/classes")

distributedTestCompile
files(project(':geode-core').buildDir.absolutePath+"/../out/test/classes")

Simply a hack to get IDEA not complaining.



On Wed, Jul 18, 2018 at 2:08 PM Jacob Barrett  wrote:

> Even maven won’t fix the issue remain in IJ. We need to refactor the code
> so that Test code does not depend on Test code. It’s just bad form to do so.
>
> -Jake
>
>
> > On Jul 18, 2018, at 1:28 PM, Kirk Lund  wrote:
> >
> > Sai had attempted to extract our testing framework(s) including DUnit to
> a
> > new geode-test module. I thought it had been merged to develop but it
> seems
> > to have been reverted. Anyone know why it had to be reverted?
> >
> > Other than moving to maven, another option would be to separate test
> types
> > based on file name: *IntegrationTest, *DistributedTest, *Test
> > (excluding *IntegrationTest,
> > *DistributedTest). It's perhaps not as elegant as separate srcSets but at
> > least Gradle and the IJ plugin can handle it properly.
> >
> >> On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett 
> wrote:
> >>
> >> The fix has been merged. It only fixes the compilation from IJ. The
> editor
> >> will continue to colorize dependencies from other test sources as
> missing.
> >> This can only be fixed by completing the extraction of the test
> framework
> >> sources into their own module. IJ editor assumes that test code should
> >> never depend on test code, which is a correct assumption but causes
> issues
> >> for us. We need to extract framework code into its own module as main
> >> source so that we can fix our gradle dependencies and correct this IJ
> >> issue.
> >>
> >> Are there any takers interested in extracting DUnit and other framework
> >> sources into their own modules?
> >>
> >>
> >>
> >>
> >> On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett 
> >> wrote:
> >>
> >>> All,
> >>>
> >>> I have a fix that appears to address all these issue in a PR. I am just
> >>> waiting for them to pass come CI because they effect the Gradle build
> >> too.
> >>> Please don’t check in any “fixes” to this IJ integration issue.
> >>>
> >>> -Jake
> >>>
> >>>
>  On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> 
>  More details: If I open up BasicDistributedTest, I can run it but all
> >> of
>  the imports for classes that are also in distributedTest are RED and
> >>> don't
>  show up in the Project window.
> 
>  After pulling, the following commit is now my head revision.
> >>> Unfortunately,
>  it doesn't fix my problem. But it did cause one change: If I search
>  for BasicDistributedTest,
>  it now finds two instances -- one is found in
> >> geode-core_distributedTest
>  and the other is found in geode-core_test.
> 
>  commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
>  *origin/develop*)
> 
>  Author: Jinmei Liao 
> 
>  Date:   Wed Jul 18 10:22:16 2018 -0700
> 
> 
>    GEODE-5363: Fixes issue with build in IJ IDEA.
> 
> 
> > On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund 
> wrote:
> >
> > Yes, my head revision is...
> >
> > commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
> > *origin/develop*, *origin/HEAD*)
> >
> > Author: Anthony Baker 
> >
> > Date:   Mon Jul 16 16:56:03 2018 -0700
> >
> >
> >   GEODE-5421 Updated dependencies
> >
> >
> >
> >   Updated bundled library dependencies. The updated libaries are:
> >
> >
> >
> >   HikariCP 3.0.0 -> 3.2.0
> >
> >   fast-classpath-scanner 2.19.0 -> 2.21
> >
> >   fastutil 8.1.1 -> 8.2.1
> >
> >   google-gson 2.8.2 -> 2.8.5
> >
> >   guava 24.1-jre -> 25.1-jre
> >
> >   jackson 2.9.5 -> 2.9.6
> >
> >   jansi 1.17 -> 1.17.1
> >
> >   netty 4.1.21.Final -> 4.1.27.Final
> >
> >   protobuf 3.5.1 -> 3.6.0
> >
> >   spring-security 4.2.4 -> 4.2.7
> >
> >   spring-* 4.3.14 -> 4.3.18
> >
> >   springfox 2.8.0 -> 2.9.2
> >
> >
> >
> >   These changes mean that javassist and reflections are no longer
> >
> >   bundled with the binary convenience distributions.
> >
> >
> >
> >
> >> On Wed, Jul 18, 2018 at 10:23 AM, Anthony Baker 
> >>> wrote:
> >>
> >> Are you using this commit?
> >>
> >> commit 89754953080cf3da9ce02a644bd3c0ac9afa1990
> >> Author: Jacob Barrett 
> >> Date:   Tue Jul 17 17:03:22 2018 -0700
> >>
> >>   GEODE-5363: Fixes issue with build in IJ IDEA.
> >

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

[Spring CI]

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

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





--
This message is automatically generated by Atlassian Bamboo

Re: How to setup IntelliJ

[Jacob Barrett]

To be clear this and editor issue not a compiler issue. Do not accidentally 
check any Gradle hacks in. To fix this someone needs to move framework code out 
of the test sources.

> On Jul 18, 2018, at 3:08 PM, Jinmei Liao  wrote:
> 
> With Jake's merge, the my integration/distributed tests in geode-cq (and
> maybe all other non-core modules) are not compiling nicely in IDEA. If you
> want a quick fix to your IDEA issue and not waiting till all the test
> refactors are done, you can add these lines in the beginning of that
> modules build.gradle's dependency section:
> 
> integrationTestCompile files(project(':geode-core').buildDir.absolutePath+
> "/../out/test/classes")
> 
> distributedTestCompile
> files(project(':geode-core').buildDir.absolutePath+"/../out/test/classes")
> 
> Simply a hack to get IDEA not complaining.
> 
> 
> 
>> On Wed, Jul 18, 2018 at 2:08 PM Jacob Barrett  wrote:
>> 
>> Even maven won’t fix the issue remain in IJ. We need to refactor the code
>> so that Test code does not depend on Test code. It’s just bad form to do so.
>> 
>> -Jake
>> 
>> 
>>> On Jul 18, 2018, at 1:28 PM, Kirk Lund  wrote:
>>> 
>>> Sai had attempted to extract our testing framework(s) including DUnit to
>> a
>>> new geode-test module. I thought it had been merged to develop but it
>> seems
>>> to have been reverted. Anyone know why it had to be reverted?
>>> 
>>> Other than moving to maven, another option would be to separate test
>> types
>>> based on file name: *IntegrationTest, *DistributedTest, *Test
>>> (excluding *IntegrationTest,
>>> *DistributedTest). It's perhaps not as elegant as separate srcSets but at
>>> least Gradle and the IJ plugin can handle it properly.
>>> 
 On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett 
>> wrote:
 
 The fix has been merged. It only fixes the compilation from IJ. The
>> editor
 will continue to colorize dependencies from other test sources as
>> missing.
 This can only be fixed by completing the extraction of the test
>> framework
 sources into their own module. IJ editor assumes that test code should
 never depend on test code, which is a correct assumption but causes
>> issues
 for us. We need to extract framework code into its own module as main
 source so that we can fix our gradle dependencies and correct this IJ
 issue.
 
 Are there any takers interested in extracting DUnit and other framework
 sources into their own modules?
 
 
 
 
 On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett 
 wrote:
 
> All,
> 
> I have a fix that appears to address all these issue in a PR. I am just
> waiting for them to pass come CI because they effect the Gradle build
 too.
> Please don’t check in any “fixes” to this IJ integration issue.
> 
> -Jake
> 
> 
>> On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
>> 
>> More details: If I open up BasicDistributedTest, I can run it but all
 of
>> the imports for classes that are also in distributedTest are RED and
> don't
>> show up in the Project window.
>> 
>> After pulling, the following commit is now my head revision.
> Unfortunately,
>> it doesn't fix my problem. But it did cause one change: If I search
>> for BasicDistributedTest,
>> it now finds two instances -- one is found in
 geode-core_distributedTest
>> and the other is found in geode-core_test.
>> 
>> commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD -> **develop*,
>> *origin/develop*)
>> 
>> Author: Jinmei Liao 
>> 
>> Date:   Wed Jul 18 10:22:16 2018 -0700
>> 
>> 
>>  GEODE-5363: Fixes issue with build in IJ IDEA.
>> 
>> 
>>> On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund 
>> wrote:
>>> 
>>> Yes, my head revision is...
>>> 
>>> commit afc8dc8fca846d08581d8027f969ceadec911687 (*HEAD -> **develop*,
>>> *origin/develop*, *origin/HEAD*)
>>> 
>>> Author: Anthony Baker 
>>> 
>>> Date:   Mon Jul 16 16:56:03 2018 -0700
>>> 
>>> 
>>>  GEODE-5421 Updated dependencies
>>> 
>>> 
>>> 
>>>  Updated bundled library dependencies. The updated libaries are:
>>> 
>>> 
>>> 
>>>  HikariCP 3.0.0 -> 3.2.0
>>> 
>>>  fast-classpath-scanner 2.19.0 -> 2.21
>>> 
>>>  fastutil 8.1.1 -> 8.2.1
>>> 
>>>  google-gson 2.8.2 -> 2.8.5
>>> 
>>>  guava 24.1-jre -> 25.1-jre
>>> 
>>>  jackson 2.9.5 -> 2.9.6
>>> 
>>>  jansi 1.17 -> 1.17.1
>>> 
>>>  netty 4.1.21.Final -> 4.1.27.Final
>>> 
>>>  protobuf 3.5.1 -> 3.6.0
>>> 
>>>  spring-security 4.2.4 -> 4.2.7
>>> 
>>>  spring-* 4.3.14 -> 4.3.18
>>> 
>>>  springfox 2.8.0 -> 2.9.2
>>> 
>>> 
>>> 
>>>  These changes mean that javassist and reflections are no longer
>>> 
>>>  bundled with the binary convenience distributions.
>>> 
>>> 
>>> 
>>> 
 

Slow DUnit tests for rolling upgrades

[Alexander Murmann]

Hi all,

We just had a conversation among some members of the community about the
increasingly slow DUnit tests that are related to rolling upgrades.

Please chime in with questions and concerns!

Here are my notes from the discussion:







*Why are rolling upgrade tests slow?Six versions of servers have to to be
1. Started with old version2. Do some work3. Shut down4. Updated5.
Restarted6. Do more work and make assertionsThe above happens for every
test method individually. This takes ~1 minute per method and per version.
This is done serially right now for all methods within a class.Our other
DUnit tests can reuse JVMs. These tests are starting a fresh JVM.How can we
make them faster?Option 1: Move every test method into its own class. This
will allow us to run them in parallelThis would allow us to run ~45 tests
in parallelMight increase ease of exploring test suite.Option 2: Move
rolling upgrade tests into their own Concourse job.This can be done in
addition to other options.Option 3: Restricting what version jumps are
supported.Can we decide to not support certain version jumps?Will we
support jumping from Geode 1.x to arbitrary Geode 2.x version?Option 4:
Change test runner to run for different version transitions in parallel.We
could generate classes that contain the desired source and destination
versions and then runs those in parallel.Next ActionsOptions 1 & 2 seem to
be fairly simple changes without much downside and should get us far.
Implementing those two solutions and seeing how long that leaves our test
suite seem like the best first step. Jacob Barrett will take the lead on
turning these two options into action.*

Re: How to setup IntelliJ

[Dan Smith]

I think there is an expectation that changes do not break the IDE as well.
Since this is a big enough pain for everyone, I think we need to do
something that fixes this issue ASAP.

Below is yet another hack that works around the issue, by telling intellij
that everything is production source code and not test sources. This means
you can't filter your searches to production code, but it does fix the
editor and it won't affect the command line build. I can check this in for
the short term while y'all work on restructuring the code if you like.

diff --git a/gradle/ide.gradle b/gradle/ide.gradle
index a8ea46d91..09541a07f 100644
--- a/gradle/ide.gradle
+++ b/gradle/ide.gradle
@@ -76,6 +76,7 @@ subprojects {
   idea {
 module {
   downloadSources = true
+  testSourceDirs = Collections.emptySet()
 }
   }
 }


On Wed, Jul 18, 2018 at 3:58 PM, Jacob Barrett  wrote:

> To be clear this and editor issue not a compiler issue. Do not
> accidentally check any Gradle hacks in. To fix this someone needs to move
> framework code out of the test sources.
>
> > On Jul 18, 2018, at 3:08 PM, Jinmei Liao  wrote:
> >
> > With Jake's merge, the my integration/distributed tests in geode-cq (and
> > maybe all other non-core modules) are not compiling nicely in IDEA. If
> you
> > want a quick fix to your IDEA issue and not waiting till all the test
> > refactors are done, you can add these lines in the beginning of that
> > modules build.gradle's dependency section:
> >
> > integrationTestCompile files(project(':geode-core').
> buildDir.absolutePath+
> > "/../out/test/classes")
> >
> > distributedTestCompile
> > files(project(':geode-core').buildDir.absolutePath+"/../
> out/test/classes")
> >
> > Simply a hack to get IDEA not complaining.
> >
> >
> >
> >> On Wed, Jul 18, 2018 at 2:08 PM Jacob Barrett 
> wrote:
> >>
> >> Even maven won’t fix the issue remain in IJ. We need to refactor the
> code
> >> so that Test code does not depend on Test code. It’s just bad form to
> do so.
> >>
> >> -Jake
> >>
> >>
> >>> On Jul 18, 2018, at 1:28 PM, Kirk Lund  wrote:
> >>>
> >>> Sai had attempted to extract our testing framework(s) including DUnit
> to
> >> a
> >>> new geode-test module. I thought it had been merged to develop but it
> >> seems
> >>> to have been reverted. Anyone know why it had to be reverted?
> >>>
> >>> Other than moving to maven, another option would be to separate test
> >> types
> >>> based on file name: *IntegrationTest, *DistributedTest, *Test
> >>> (excluding *IntegrationTest,
> >>> *DistributedTest). It's perhaps not as elegant as separate srcSets but
> at
> >>> least Gradle and the IJ plugin can handle it properly.
> >>>
>  On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett 
> >> wrote:
> 
>  The fix has been merged. It only fixes the compilation from IJ. The
> >> editor
>  will continue to colorize dependencies from other test sources as
> >> missing.
>  This can only be fixed by completing the extraction of the test
> >> framework
>  sources into their own module. IJ editor assumes that test code should
>  never depend on test code, which is a correct assumption but causes
> >> issues
>  for us. We need to extract framework code into its own module as main
>  source so that we can fix our gradle dependencies and correct this IJ
>  issue.
> 
>  Are there any takers interested in extracting DUnit and other
> framework
>  sources into their own modules?
> 
> 
> 
> 
>  On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett 
>  wrote:
> 
> > All,
> >
> > I have a fix that appears to address all these issue in a PR. I am
> just
> > waiting for them to pass come CI because they effect the Gradle build
>  too.
> > Please don’t check in any “fixes” to this IJ integration issue.
> >
> > -Jake
> >
> >
> >> On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> >>
> >> More details: If I open up BasicDistributedTest, I can run it but
> all
>  of
> >> the imports for classes that are also in distributedTest are RED and
> > don't
> >> show up in the Project window.
> >>
> >> After pulling, the following commit is now my head revision.
> > Unfortunately,
> >> it doesn't fix my problem. But it did cause one change: If I search
> >> for BasicDistributedTest,
> >> it now finds two instances -- one is found in
>  geode-core_distributedTest
> >> and the other is found in geode-core_test.
> >>
> >> commit 47932e85cc7dc76aa00a667552d1c0fc3fe52b85 (*HEAD ->
> **develop*,
> >> *origin/develop*)
> >>
> >> Author: Jinmei Liao 
> >>
> >> Date:   Wed Jul 18 10:22:16 2018 -0700
> >>
> >>
> >>  GEODE-5363: Fixes issue with build in IJ IDEA.
> >>
> >>
> >>> On Wed, Jul 18, 2018 at 10:29 AM, Kirk Lund 
> >> wrote:
> >>>
> >>> Yes, my head revision is...
> >>>
> >>> commit afc8dc8fca846d08581d8027f969ce

Re: Slow DUnit tests for rolling upgrades

[Dan Smith]

Can you fix the formatting of your message? Seems to have gotten mangled.

On Wed, Jul 18, 2018 at 4:20 PM, Alexander Murmann 
wrote:

> Hi all,
>
> We just had a conversation among some members of the community about the
> increasingly slow DUnit tests that are related to rolling upgrades.
>
> Please chime in with questions and concerns!
>
> Here are my notes from the discussion:
>
>
>
>
>
>
>
> *Why are rolling upgrade tests slow?Six versions of servers have to to be
> 1. Started with old version2. Do some work3. Shut down4. Updated5.
> Restarted6. Do more work and make assertionsThe above happens for every
> test method individually. This takes ~1 minute per method and per version.
> This is done serially right now for all methods within a class.Our other
> DUnit tests can reuse JVMs. These tests are starting a fresh JVM.How can we
> make them faster?Option 1: Move every test method into its own class. This
> will allow us to run them in parallelThis would allow us to run ~45 tests
> in parallelMight increase ease of exploring test suite.Option 2: Move
> rolling upgrade tests into their own Concourse job.This can be done in
> addition to other options.Option 3: Restricting what version jumps are
> supported.Can we decide to not support certain version jumps?Will we
> support jumping from Geode 1.x to arbitrary Geode 2.x version?Option 4:
> Change test runner to run for different version transitions in parallel.We
> could generate classes that contain the desired source and destination
> versions and then runs those in parallel.Next ActionsOptions 1 & 2 seem to
> be fairly simple changes without much downside and should get us far.
> Implementing those two solutions and seeing how long that leaves our test
> suite seem like the best first step. Jacob Barrett will take the lead on
> turning these two options into action.*
>

Re: Slow DUnit tests for rolling upgrades

[Alexander Murmann]

Thanks for catching this, Dan! Not sure what happened there.
Let's see if this works better:

Why are rolling upgrade tests slow?
==
Six versions of servers have to to be
1. Started with old version
2. Do some work
3. Shut down
4. Updated
5. Restarted
6. Do more work and make assertions

The above happens for every test method individually. This takes ~1 minute
per method and per version. This is done serially right now for all methods
within a class.
Our other DUnit tests can reuse JVMs. These tests are starting a fresh JVM.

How can we make them faster?

Option 1: Move every test method into its own class. This will allow us to
run them in parallel
This would allow us to run ~45 tests in parallel
Might increase ease of exploring test suite.

Option 2: Move rolling upgrade tests into their own Concourse job.
This can be done in addition to other options.

Option 3: Restricting what version jumps are supported.
Can we decide to not support certain version jumps?
Will we support jumping from Geode 1.x to arbitrary Geode 2.x version?

Option 4: Change test runner to run for different version transitions in
parallel.
We could generate classes that contain the desired source and destination
versions and then runs those in parallel.

Next Actions
===
Options 1 & 2 seem to be fairly simple changes without much downside and
should get us far. Implementing those two solutions and seeing how long
that leaves our test suite seem like the best first step.
Jacob Barrett will take the lead on turning these two options into action.


On Wed, Jul 18, 2018 at 5:04 PM, Dan Smith  wrote:

> Can you fix the formatting of your message? Seems to have gotten mangled.
>
> On Wed, Jul 18, 2018 at 4:20 PM, Alexander Murmann 
> wrote:
>
> > Hi all,
> >
> > We just had a conversation among some members of the community about the
> > increasingly slow DUnit tests that are related to rolling upgrades.
> >
> > Please chime in with questions and concerns!
> >
> > Here are my notes from the discussion:
> >
> >
> >
> >
> >
> >
> >
> > *Why are rolling upgrade tests slow?Six versions of servers have to to be
> > 1. Started with old version2. Do some work3. Shut down4. Updated5.
> > Restarted6. Do more work and make assertionsThe above happens for every
> > test method individually. This takes ~1 minute per method and per
> version.
> > This is done serially right now for all methods within a class.Our other
> > DUnit tests can reuse JVMs. These tests are starting a fresh JVM.How can
> we
> > make them faster?Option 1: Move every test method into its own class.
> This
> > will allow us to run them in parallelThis would allow us to run ~45 tests
> > in parallelMight increase ease of exploring test suite.Option 2: Move
> > rolling upgrade tests into their own Concourse job.This can be done in
> > addition to other options.Option 3: Restricting what version jumps are
> > supported.Can we decide to not support certain version jumps?Will we
> > support jumping from Geode 1.x to arbitrary Geode 2.x version?Option 4:
> > Change test runner to run for different version transitions in
> parallel.We
> > could generate classes that contain the desired source and destination
> > versions and then runs those in parallel.Next ActionsOptions 1 & 2 seem
> to
> > be fairly simple changes without much downside and should get us far.
> > Implementing those two solutions and seeing how long that leaves our test
> > suite seem like the best first step. Jacob Barrett will take the lead on
> > turning these two options into action.*
> >
>

Geode unit tests completed in 'develop/DistributedTest' with non-zero exit code

[apachegeodeci]

Pipeline results can be found at:

Concourse: 
https://concourse.apachegeode-ci.info/teams/main/pipelines/develop/jobs/DistributedTest/builds/126

Re: How to setup IntelliJ

[Jinmei Liao]

+1 for Dan's change. I cant stand IntelliJ showing me any red lines, even
though compiler works.

On Wed, Jul 18, 2018, 5:00 PM Dan Smith  wrote:

> I think there is an expectation that changes do not break the IDE as well.
> Since this is a big enough pain for everyone, I think we need to do
> something that fixes this issue ASAP.
>
> Below is yet another hack that works around the issue, by telling intellij
> that everything is production source code and not test sources. This means
> you can't filter your searches to production code, but it does fix the
> editor and it won't affect the command line build. I can check this in for
> the short term while y'all work on restructuring the code if you like.
>
> diff --git a/gradle/ide.gradle b/gradle/ide.gradle
> index a8ea46d91..09541a07f 100644
> --- a/gradle/ide.gradle
> +++ b/gradle/ide.gradle
> @@ -76,6 +76,7 @@ subprojects {
>idea {
>  module {
>downloadSources = true
> +  testSourceDirs = Collections.emptySet()
>  }
>}
>  }
>
>
> On Wed, Jul 18, 2018 at 3:58 PM, Jacob Barrett 
> wrote:
>
> > To be clear this and editor issue not a compiler issue. Do not
> > accidentally check any Gradle hacks in. To fix this someone needs to move
> > framework code out of the test sources.
> >
> > > On Jul 18, 2018, at 3:08 PM, Jinmei Liao  wrote:
> > >
> > > With Jake's merge, the my integration/distributed tests in geode-cq
> (and
> > > maybe all other non-core modules) are not compiling nicely in IDEA. If
> > you
> > > want a quick fix to your IDEA issue and not waiting till all the test
> > > refactors are done, you can add these lines in the beginning of that
> > > modules build.gradle's dependency section:
> > >
> > > integrationTestCompile files(project(':geode-core').
> > buildDir.absolutePath+
> > > "/../out/test/classes")
> > >
> > > distributedTestCompile
> > > files(project(':geode-core').buildDir.absolutePath+"/../
> > out/test/classes")
> > >
> > > Simply a hack to get IDEA not complaining.
> > >
> > >
> > >
> > >> On Wed, Jul 18, 2018 at 2:08 PM Jacob Barrett 
> > wrote:
> > >>
> > >> Even maven won’t fix the issue remain in IJ. We need to refactor the
> > code
> > >> so that Test code does not depend on Test code. It’s just bad form to
> > do so.
> > >>
> > >> -Jake
> > >>
> > >>
> > >>> On Jul 18, 2018, at 1:28 PM, Kirk Lund  wrote:
> > >>>
> > >>> Sai had attempted to extract our testing framework(s) including DUnit
> > to
> > >> a
> > >>> new geode-test module. I thought it had been merged to develop but it
> > >> seems
> > >>> to have been reverted. Anyone know why it had to be reverted?
> > >>>
> > >>> Other than moving to maven, another option would be to separate test
> > >> types
> > >>> based on file name: *IntegrationTest, *DistributedTest, *Test
> > >>> (excluding *IntegrationTest,
> > >>> *DistributedTest). It's perhaps not as elegant as separate srcSets
> but
> > at
> > >>> least Gradle and the IJ plugin can handle it properly.
> > >>>
> >  On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett  >
> > >> wrote:
> > 
> >  The fix has been merged. It only fixes the compilation from IJ. The
> > >> editor
> >  will continue to colorize dependencies from other test sources as
> > >> missing.
> >  This can only be fixed by completing the extraction of the test
> > >> framework
> >  sources into their own module. IJ editor assumes that test code
> should
> >  never depend on test code, which is a correct assumption but causes
> > >> issues
> >  for us. We need to extract framework code into its own module as
> main
> >  source so that we can fix our gradle dependencies and correct this
> IJ
> >  issue.
> > 
> >  Are there any takers interested in extracting DUnit and other
> > framework
> >  sources into their own modules?
> > 
> > 
> > 
> > 
> >  On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett  >
> >  wrote:
> > 
> > > All,
> > >
> > > I have a fix that appears to address all these issue in a PR. I am
> > just
> > > waiting for them to pass come CI because they effect the Gradle
> build
> >  too.
> > > Please don’t check in any “fixes” to this IJ integration issue.
> > >
> > > -Jake
> > >
> > >
> > >> On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> > >>
> > >> More details: If I open up BasicDistributedTest, I can run it but
> > all
> >  of
> > >> the imports for classes that are also in distributedTest are RED
> and
> > > don't
> > >> show up in the Project window.
> > >>
> > >> After pulling, the following commit is now my head revision.
> > > Unfortunately,
> > >> it doesn't fix my problem. But it did cause one change: If I
> search
> > >> for BasicDistributedTest,
> > >> it now finds two instances -- one is found in
> >  geode-core_distributedTest
> > >> and the other is found in geode-core_test.
> > >>
> > >> commit 47932e85cc7dc76aa00a6

Assertions in Geode

[Galen O'Sullivan]

Hi all,

I'm wondering what the collective's opinion of assertions is. I notice
there's an org.apache.geode.internal.Assert class, which is used some
places, and that plain old Java assertions are used in other places. Can we
remove one of these and use the other? Should we be including assertions in
new or refactored code?

Thanks,
Galen

Re: Assertions in Geode

[Jinmei Liao]

dont know what other people do, but I do not use assertions in production
code. in test code, I like to use assertJ whenever possible since it
provide more convenience methods and more descriptive failure messages than
junit assert.

On Wed, Jul 18, 2018, 6:52 PM Galen O'Sullivan 
wrote:

> Hi all,
>
> I'm wondering what the collective's opinion of assertions is. I notice
> there's an org.apache.geode.internal.Assert class, which is used some
> places, and that plain old Java assertions are used in other places. Can we
> remove one of these and use the other? Should we be including assertions in
> new or refactored code?
>
> Thanks,
> Galen
>

Re: How to setup IntelliJ

[Jacob Barrett]

This patch makes the editor work and breaks the compiler. ;) So it inverts the 
original problem that IJ compiles Test on Test dependencies but restricts them 
in the editor.

The fix continues to be to write proper tests that don’t depend on other tests. 
I’ve started breaking out DUnit, which fixes the bulk of the editor issues.

-Jake


> On Jul 18, 2018, at 6:31 PM, Jinmei Liao  wrote:
> 
> +1 for Dan's change. I cant stand IntelliJ showing me any red lines, even
> though compiler works.
> 
>> On Wed, Jul 18, 2018, 5:00 PM Dan Smith  wrote:
>> 
>> I think there is an expectation that changes do not break the IDE as well.
>> Since this is a big enough pain for everyone, I think we need to do
>> something that fixes this issue ASAP.
>> 
>> Below is yet another hack that works around the issue, by telling intellij
>> that everything is production source code and not test sources. This means
>> you can't filter your searches to production code, but it does fix the
>> editor and it won't affect the command line build. I can check this in for
>> the short term while y'all work on restructuring the code if you like.
>> 
>> diff --git a/gradle/ide.gradle b/gradle/ide.gradle
>> index a8ea46d91..09541a07f 100644
>> --- a/gradle/ide.gradle
>> +++ b/gradle/ide.gradle
>> @@ -76,6 +76,7 @@ subprojects {
>>   idea {
>> module {
>>   downloadSources = true
>> +  testSourceDirs = Collections.emptySet()
>> }
>>   }
>> }
>> 
>> 
>> On Wed, Jul 18, 2018 at 3:58 PM, Jacob Barrett 
>> wrote:
>> 
>>> To be clear this and editor issue not a compiler issue. Do not
>>> accidentally check any Gradle hacks in. To fix this someone needs to move
>>> framework code out of the test sources.
>>> 
 On Jul 18, 2018, at 3:08 PM, Jinmei Liao  wrote:
 
 With Jake's merge, the my integration/distributed tests in geode-cq
>> (and
 maybe all other non-core modules) are not compiling nicely in IDEA. If
>>> you
 want a quick fix to your IDEA issue and not waiting till all the test
 refactors are done, you can add these lines in the beginning of that
 modules build.gradle's dependency section:
 
 integrationTestCompile files(project(':geode-core').
>>> buildDir.absolutePath+
 "/../out/test/classes")
 
 distributedTestCompile
 files(project(':geode-core').buildDir.absolutePath+"/../
>>> out/test/classes")
 
 Simply a hack to get IDEA not complaining.
 
 
 
> On Wed, Jul 18, 2018 at 2:08 PM Jacob Barrett 
>>> wrote:
> 
> Even maven won’t fix the issue remain in IJ. We need to refactor the
>>> code
> so that Test code does not depend on Test code. It’s just bad form to
>>> do so.
> 
> -Jake
> 
> 
>> On Jul 18, 2018, at 1:28 PM, Kirk Lund  wrote:
>> 
>> Sai had attempted to extract our testing framework(s) including DUnit
>>> to
> a
>> new geode-test module. I thought it had been merged to develop but it
> seems
>> to have been reverted. Anyone know why it had to be reverted?
>> 
>> Other than moving to maven, another option would be to separate test
> types
>> based on file name: *IntegrationTest, *DistributedTest, *Test
>> (excluding *IntegrationTest,
>> *DistributedTest). It's perhaps not as elegant as separate srcSets
>> but
>>> at
>> least Gradle and the IJ plugin can handle it properly.
>> 
>>> On Wed, Jul 18, 2018 at 1:11 PM, Jacob Barrett >> 
> wrote:
>>> 
>>> The fix has been merged. It only fixes the compilation from IJ. The
> editor
>>> will continue to colorize dependencies from other test sources as
> missing.
>>> This can only be fixed by completing the extraction of the test
> framework
>>> sources into their own module. IJ editor assumes that test code
>> should
>>> never depend on test code, which is a correct assumption but causes
> issues
>>> for us. We need to extract framework code into its own module as
>> main
>>> source so that we can fix our gradle dependencies and correct this
>> IJ
>>> issue.
>>> 
>>> Are there any takers interested in extracting DUnit and other
>>> framework
>>> sources into their own modules?
>>> 
>>> 
>>> 
>>> 
>>> On Wed, Jul 18, 2018 at 12:18 PM Jacob Barrett >> 
>>> wrote:
>>> 
 All,
 
 I have a fix that appears to address all these issue in a PR. I am
>>> just
 waiting for them to pass come CI because they effect the Gradle
>> build
>>> too.
 Please don’t check in any “fixes” to this IJ integration issue.
 
 -Jake
 
 
> On Jul 18, 2018, at 10:54 AM, Kirk Lund  wrote:
> 
> More details: If I open up BasicDistributedTest, I can run it but
>>> all
>>> of
> the imports for classes that are also in distributedTest are RED
>> and
 don't
> show up in the Project window.
> 
> After pulling, 

Re: Assertions in Geode

[Jacob Barrett]

There is a HUGE difference between Java language assert and an class called 
Assert.

 Java language asserts are disabled at runtime by default. They should only be 
used for testing assertions when running in a “Test” mode. Since under such 
conditions you should have good unit test doing the same it seems redundant to 
even have them in the code. 

Under what conditions are you seeing both types of assertions being used in the 
main code?

> On Jul 18, 2018, at 6:52 PM, Galen O'Sullivan  wrote:
> 
> Hi all,
> 
> I'm wondering what the collective's opinion of assertions is. I notice
> there's an org.apache.geode.internal.Assert class, which is used some
> places, and that plain old Java assertions are used in other places. Can we
> remove one of these and use the other? Should we be including assertions in
> new or refactored code?
> 
> Thanks,
> Galen

Re: How to setup IntelliJ

[Jacob Barrett]

Checkout the latest from develop.
Configure IntelliJ Build / Build Tools / Gradle / Uncheck "Create separate
module per source set".
Refresh the Gradle config (or enable auto import).
Compile!

-Jake