geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jens Deppe <jde...@pivotal.io>
Subject Re: [DISCUSS]: Tests requiring external services
Date Thu, 05 Apr 2018 13:50:32 GMT
I would be hesitant to start categorizing tests by name (again) as we'd be
introducing a different (non consistent) qualifier which is only relevant
to one class of tests.

All AcceptanceTests should already be grouped in the geode-assembly module
so running them with geode-assembly:acceptanceTest should limit the
performance impact - as opposed to running the acceptanceTest task across
the whole project.

--Jens

On Wed, Apr 4, 2018 at 4:42 PM, Kirk Lund <klund@apache.org> wrote:

> Nick made the changes necessary to open up AcceptanceTest to other Geode
> submodules and it takes too long. The cause is what Jared mentioned.
> Basically, Gradle forks a new JVM for every *Test in our code base and then
> executes JUnit which queries the test class for Category and either runs or
> doesn't run.
>
> I propose renaming all tests with AcceptanceTest Category to
> *AcceptanceTest and then make one more change to Nick's branch to use
> *AcceptanceTest file pattern in any submodule. Assuming no one disagrees
> with this or has a better idea, I'll try it out in his branch tomorrow.
>
> On Wed, Apr 4, 2018 at 10:09 AM, Jared Stewart <stewart.jared@gmail.com>
> wrote:
>
> > Just FYI, the reason that :acceptanceTest is currently only a target of
> > precheckin is https://issues.apache.org/jira/browse/GEODE-3296
> >
> > For the full details, see this thread on the Gradle Forums:
> > https://discuss.gradle.org/t/test-task-with-forkevery-1-
> > and-includecategories-performs-poorly/23401
> >
> >
> > On Wed, Apr 4, 2018 at 9:56 AM, Patrick Rhomberg <prhomberg@pivotal.io>
> > wrote:
> >
> > > +1.  AcceptanceTest seems fittings, although...
> > >
> > > That test category was created with the focus on tests that run gfsh
> > > scripts via the GfshRule.  Because the GfshRule uses the built jar and
> > > actually launches gfsh to run its tests, all current AcceptanceTests
> > exist
> > > in geode-assembly.  Perhaps as an oversight, only
> > > :geode-assembly:acceptanceTest is a target of the precheckin task.
> > >
> > > If we want to expand the scope of the AcceptanceTest tag, we'll need to
> > go
> > > update the gradle to make sure these tests get picked up.
> > >
> > > On Wed, Apr 4, 2018 at 9:39 AM, Kirk Lund <klund@apache.org> wrote:
> > >
> > > > +1 to using AcceptanceTest category for the end-to-end JDBC connector
> > > > service tests
> > > >
> > > > On Wed, Apr 4, 2018 at 8:41 AM, Nick Reich <nreich@pivotal.io>
> wrote:
> > > >
> > > > > Using AcceptanceTest category seems like a good solution at the
> > moment
> > > to
> > > > > me.
> > > > >
> > > > > On Tue, Apr 3, 2018 at 4:29 PM, Sean Goller <sgoller@pivotal.io>
> > > wrote:
> > > > >
> > > > > > I'm actually fine with putting it in AcceptanceTest for now.
> > > > > >
> > > > > > Ideally I'd like to see something like JDBC connection strings
> that
> > > > could
> > > > > > be passed in as properties via the command-line, and if they're
> not
> > > > > present
> > > > > > the relevant tests don't get run. That way the entity running
the
> > > tests
> > > > > can
> > > > > > decide the best way to enable those tests.
> > > > > >
> > > > > > On Tue, Apr 3, 2018 at 4:11 PM, Jens Deppe <jdeppe@pivotal.io>
> > > wrote:
> > > > > >
> > > > > > > I'm in favor of using docker for test isolation. We already
> have
> > an
> > > > > > > 'AcceptanceTest' category which you might consider using
> instead
> > of
> > > > > > > creating yet another category.
> > > > > > >
> > > > > > > --Jens
> > > > > > >
> > > > > > > On Tue, Apr 3, 2018 at 2:02 PM, Nick Reich <nreich@pivotal.io>
> > > > wrote:
> > > > > > >
> > > > > > > > Team,
> > > > > > > >
> > > > > > > > As part of validating the new JDBC connector for Geode,
we
> > have a
> > > > > need
> > > > > > > for
> > > > > > > > tests that involving connecting to specific databases
(like
> > MySQL
> > > > and
> > > > > > > > Postgres) to validate proper function with those databases.
> > Since
> > > > > these
> > > > > > > > tests require connecting to outside services, unlike
existing
> > > Geode
> > > > > > > tests,
> > > > > > > > we are seeking suggestions on how to best incorporate
such
> > tests
> > > > into
> > > > > > > > Geode. The approach we have taken so far is to utilize
Docker
> > > (and
> > > > > > Docker
> > > > > > > > Compose) to take care of spinning up our external
services
> for
> > > the
> > > > > > > duration
> > > > > > > > of the tests. This, however requires that Docker and
Docker
> > > Compose
> > > > > be
> > > > > > > > installed on any machine that the tests are run on.
> > Additionally,
> > > > the
> > > > > > > > Concourse pipeline for validating develop is incompatible
> with
> > > use
> > > > of
> > > > > > > > Docker for distributed tests, due to the way they
are already
> > > being
> > > > > run
> > > > > > > > within Docker containers of their own (it seems possible
to
> > make
> > > it
> > > > > > work,
> > > > > > > > but would add overhead to all tests and would be a
challenge
> to
> > > > > > > implement).
> > > > > > > >
> > > > > > > > To address these issues, we are considering having
these
> tests
> > > run
> > > > > > under
> > > > > > > a
> > > > > > > > new task, such as "serviceTest" (instead of IntegrationTest
> or
> > > > > > > > distributedTest). That way, developers could run all
other
> > tests
> > > > > > normally
> > > > > > > > on their machines, only requiring Docker and Docker
Compose
> if
> > > they
> > > > > > wish
> > > > > > > to
> > > > > > > > run these specific tests. This would also allow them
to be
> > their
> > > > own
> > > > > > task
> > > > > > > > in Concourse, eliminating the issues that plague integrating
> > > these
> > > > > > tests
> > > > > > > > there.
> > > > > > > >
> > > > > > > > Are there other ideas on how to manage these tests
or
> concerns
> > > with
> > > > > the
> > > > > > > > proposed approach?
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message