[
https://issues.apache.org/jira/browse/GEODE-5874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dale Emery updated GEODE-5874:
------------------------------
Description:
The {{org.apache.geode.cache.query.data.Position}} class uses a static counter
to supply unique identifiers to distinguish instances. One of the class’s
constructors increments the counter each time it is invoked.
Because the counter is declared {{static}}, its value persists from one test
method to the next. This causes some tests to depend on being run in a specific
order. It also causes some tests fail when run multiple times, such as when run
as part the stress test job triggered by pull request pre-checkin.
Other data classes in the {{org.apache.geode.cache.query.data}} package amplify
the problem by relying on {{Position}}'s static counter. For example, the
{{Portfolio}} class in the same package uses {{Position}}’s static counter to
select which data to construct.
Given that some tests depend on the {{Portfolio}} constructor’s choice of data,
this makes many uses of {{Portfolio}} dependent on the order in which tests are
run.
For example: {{QueryJUnitTest.test008NullCollectionField()}} failed 38 out of
50 times in a recent stress test, because on many runs, {{Portfolio}} created
an unexpected set of data:
http://files.apachegeode-ci.info/builds/geode-pr-2601/test-results/repeatTest/1539309680/
Though {{QueryJunitTest}} has been fixed to prevent this problem, other tests
remain susceptible to the problem, and will fail the next time they are run
repeatedly.
I recommend implementing a better means to create {{Position}} instances with
unique identifiers, without making tests depend on the order in which they are
run.
A Builder might be an appropriate pattern to use. Or something simpler:
Changing the {{Position}} constructor to take a unique identifier as a
parameter, as the {{Portfolio}} constructor does.
was:
The {{org.apache.geode.cache.query.data.Position}} class uses a static counter
to supply unique identifiers to distinguish instances. One of the class’s
constructors increments the counter each time it is invoked.
Because the counter is declared {{static}}, its value persists from one test
method to the next. This causes some tests to depend on being run in a specific
order. It also causes some tests fail when run multiple times, such as when run
as part the stress test job triggered by pull request pre-checkin.
Other data classes in the {{org.apache.geode.cache.query.data}} package amplify
the problem by relying on {{Position}}'s static counter. For example, the
{{Portfolio}} class in the same package uses {{Position}}’s static counter to
select which data to construct.
Given that some tests depend on the {{Portfolio}} constructor’s choice of data,
this makes many uses of {{Portfolio}} dependent on the order in which tests are
run.
For example: {{QueryJUnitTest.test008NullCollectionField()}} failed 38 out of
50 times in a recent stress test, because on many runs, {{Portfolio}} created
an unexpected set of data:
http://files.apachegeode-ci.info/builds/geode-pr-2601/test-results/repeatTest/1539309680/
Though {{QueryJunitTest}} has been fixed to prevent this problem, other tests
remain susceptible to the problem, and will fail the next time they are run
repeatedly.
I recommend implementing a better means to create {{Position}} instances with
unique identifiers, without making tests dependent on the order in which they
are run.
A Builder might be an appropriate pattern to use. Or something simpler:
Changing the {{Position}} constructor to take a unique identifier as a
parameter, as the {{Portfolio}} constructor does.
> Query test data classes cause tests to depend on run order
> ----------------------------------------------------------
>
> Key: GEODE-5874
> URL: https://issues.apache.org/jira/browse/GEODE-5874
> Project: Geode
> Issue Type: Test
> Components: tests
> Reporter: Dale Emery
> Priority: Major
> Labels: swat
>
> The {{org.apache.geode.cache.query.data.Position}} class uses a static
> counter to supply unique identifiers to distinguish instances. One of the
> class’s constructors increments the counter each time it is invoked.
> Because the counter is declared {{static}}, its value persists from one test
> method to the next. This causes some tests to depend on being run in a
> specific order. It also causes some tests fail when run multiple times, such
> as when run as part the stress test job triggered by pull request pre-checkin.
> Other data classes in the {{org.apache.geode.cache.query.data}} package
> amplify the problem by relying on {{Position}}'s static counter. For example,
> the {{Portfolio}} class in the same package uses {{Position}}’s static
> counter to select which data to construct.
> Given that some tests depend on the {{Portfolio}} constructor’s choice of
> data, this makes many uses of {{Portfolio}} dependent on the order in which
> tests are run.
> For example: {{QueryJUnitTest.test008NullCollectionField()}} failed 38 out of
> 50 times in a recent stress test, because on many runs, {{Portfolio}} created
> an unexpected set of data:
> http://files.apachegeode-ci.info/builds/geode-pr-2601/test-results/repeatTest/1539309680/
> Though {{QueryJunitTest}} has been fixed to prevent this problem, other tests
> remain susceptible to the problem, and will fail the next time they are run
> repeatedly.
> I recommend implementing a better means to create {{Position}} instances with
> unique identifiers, without making tests depend on the order in which they
> are run.
>
> A Builder might be an appropriate pattern to use. Or something simpler:
> Changing the {{Position}} constructor to take a unique identifier as a
> parameter, as the {{Portfolio}} constructor does.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
