For testing functionality like 'gfsh deploy' one has to be aware of
classloading issues. Specifically, when writing dunit tests, classes are
compiled from java resource files so that they are guaranteed not to be on
the classpath during testing. This is less of an issue for gfsh acceptance
tests where there is more control over the classpath and what is launched
as part of the test.
For the dunit deploy testing scenario (and any similar) I would prefer to
see the java test resource as exactly that - a package-equivalent resource
file that is explicitly compiled as part of the test. It makes the test
clearer and doesn't leave anyone wondering where/how resources are built or
injected.
--Jens
On Fri, Oct 12, 2018 at 4:22 PM Kirk Lund <kl...@apache.org> wrote:
I usually create a private static inner class and keep it as small as
possible at the end of the test class that uses it. Some of these these
such as MyCacheListener can be replaced by spy(CacheListener.class) and I
always try to convert these where possible.
If it's a simple class that only tests in one package will use (like a
custom AssertJ class that nothing outside that package will use) then I
make it package-protected and stick it in the same package and srcSet as
the tests that will use it.
I think something like MonthBasedPartitionResolver is simple enough that
it
should exist within the same src set and in the same package as the test
or
a few tests that use it. And if another test in some other src set or in
another package needs something similar, then too bad, that other test
should create its own simple small class that is similar. This sort of
class does NOT belong in a src/main of some module like geode-dunit.
Better
yet make it a static inner class in every test that uses it. I know some
people object to this, but here's why...
If I want to modify one test that uses MonthBasedPartitionResolver and
this
modification involves changing MonthBasedPartitionResolver (which
remember
it's a simple class) then if each test has its own copy I won't risk
breaking other tests. When I'm fixing up, cleaning up and refactoring
tests, the biggest nightmares I run into are shared utility classes such
that if I try to cleanup one test, I can't -- I have to clean up
potentially dozens of tests because they linked some some stupid utility
class because "code sharing" is great. MonthBasedPartitionResolver is a
small example, there are plenty of other examples that are true
monstrosities.
Sharing code between tests is NOT great unless you devise a Rule or
custom
AssertJ Assertion that is truly very reusable across many tests -- and
only
in this case it belongs in geode-junit or geode-dunit where it should
live
in a src/main and there should be tests for it under src/test or
src/integrationTest. I also highly recommend that you keep shared testing
objects like this small in scope with a single high-level responsibility.
And please, never create something that simply creates a new improved API
so that you don't have to use the Geode API -- this just promotes leaving
crappy product APIs in place without changing them because by using some
fancy testing API, we don't feel the pain that Users experience with the
product APIs.
On Fri, Oct 12, 2018 at 3:32 PM, Patrick Rhomberg <prhomb...@apache.org>
wrote:
Hello, all!
There are a number of classes that require some number of "toy"
classes
as part of their testing framework, e.g., to be used as custom data
containers. Many of these classes involve testing or use of the
`deploy
jar` command, and so are packaged into jars for this purpose.
In an effort to promote good coding habits and, as importantly,
consensus
throughout this community and our codebase, I would like to ask which
of
these are considered the preferred method to maintain these additional
classes.
I realize a priori that none of these options will be applicable in
all
cases, but am curious of how the prioritization of these options are
ordered among you all.
-- Class definition --
For definition of the classes consumed, we observe all of the following
in
the Geode codebase.
Option C-1: Toy classes are defined as a proper class in a reasonable
package.
-- suboption (a): The class is defined in the module in which it is
consumed, as a resource. [1]
-- suboption (b): The class is defined in the module in which it is
consumed, as a neighboring file. [2]
-- suboption (c): The class is defined in geode-dunit or geode-junit.
[3]
Option C-2: Toy classes are defined as inner classes of the test
class.
[4]
Option C-3: Sufficiently small toy classes are defined as raw String
fields in the test class that consumed it and is compiled by the test
JVM
via the JavaCompiler class. [5]
-- Resource exposure --
For those tests that require classes placed in Jars for use in the
`deploy
jar` command, we observe the following in the Geode codebase:
Option J-1: Jars are a resource and are built or downloaded as
necessary
during the build steps, before test execution. [6]
Option J-2: Sufficiently small classes that are defined as raw String
fields in the test class that consumes it (C-3 above) are packaged into
resources via the JarBuilder class. [7]
I look forward to hearing your opinions.
Imagination is Change.
~Patrick Rhomberg
Examples:
[1]
geode-core_test/resources:org.apache.geode.management.internal.deployment.
ConcreteExtendsAbstractExtendsFunctionAdapter.java
is found by the FunctionScannerTest to be a user-defined function.
[2]
geode-core_distributedTest:org.apache.geode.internal.
cache.SerializableMonth
defines a simple DataSerializable containing an int for the month, to
be
consumed by *.MonthBasedPartitionResolver
in PRCustomPartitioningDistributedTest.
[3] geode-junit_main:org.apache.geode.cache.query.transaction.Person
defines a simple DataSerilazable container class for a String name and
int
age.
[4]
geode-core_distributedTest:java.org.apache.geode.internal.statistics.
StatisticsDistributedTest$PubSubStats
is consumed by its parent class
[5]
geode-assembly_acceptanceTest:org.apache.geode.management.
internal.cli.commands.PutCommandWithJsonTest
defines a Customer class in a raw string in the test's @Before method.
[6] See our own consumption of geode-dependencies.jar et al via the
StartMemberUtils.java, or our historic consumption of tools.jar
[7] See again [5] or other uses of the JarBuilder class.
