You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/09/22 17:46:40 UTC
[GitHub] [geode] demery-pivotal opened a new pull request #6890: GEODE-4181: Migrate uses of JUnitParamsRunner
demery-pivotal opened a new pull request #6890:
URL: https://github.com/apache/geode/pull/6890
Migrate all uses of `JUnitParamsRunner` to use a new runner that can be
made compatible with JUnit 5.
PROBLEM
We want to upgrade Geode to use JUnit 5. JUnit 5 includes a test engine
called JUnit Vintage, which it uses to run JUnit 4 tests. JUnit Vintage
interacts with test runners differently than JUnit 4 did.
`JUnitParamsRunner` is incompatible with JUnit Vintage in several ways:
- When `JUnitParamsRunner` describes parameterized tests, it discards
the `@Category` annotation that JUnit Vintage uses to filter tests by
category.
- When JUnit Vintage asks `JUnitParamsRunner` to filter tests by name,
the runner fails to apply the filter to parameterized tests.
The net result is that when JUnit Vintage tries to filter tests by
category, `JUnitParamsRunner` runs every parameterized test, including
tests that lack the category. This affects the Windows Gfsh distributed
test job in particular, causing it to run 900 tests instead of the
desired 200. Numerous of the extra 700 tests fail on Windows.
This problem prevents us from upgrading to JUnit 5.
`JUnitParamsRunner` is no longer maintained. It will not be made
compatible with JUnit Vintage.
Approximately 140 test classes use `JUnitParamsRunner`. To upgrade to
JUnit 5, we will have to migrate all of these tests to use a runner
compatible with JUnit Vintage.
SOLUTION
The solution involves two steps:
1. Migrate every existing use of `JUnitParamsRunner` to instead use a
new `GeodeParamsRunner` that behaves identically to
`JUnitParamsRunner`.
2. Upgrade Geode to use JUnit 5.
This PR implements step 1. This PR implements a "dummy" version of
`GeodeParamsRunner` that behaves identically to `JUnitParamsRunner`. Its
only purpose is to allow migrating approximately 140 test classes to use
the new runner name.
I will implement step 2 in a later PR. This will include re-implementing
`GeodeParamsRunner` to be compatible with JUnit 5. (I have verified the
viability of this via a spike.)
Separating the "migrate the tests" and "upgrade JUnit" steps into
separate PRs makes each PR much more focused and much easier to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jchen21 commented on pull request #6890: GEODE-4181: Migrate uses of JUnitParamsRunner
Posted by GitBox <gi...@apache.org>.
jchen21 commented on pull request #6890:
URL: https://github.com/apache/geode/pull/6890#issuecomment-926051783
Great introduction of the pull request. Really appreciate that. Will this pull request introduce some temporary failures before another step 2 (re-implementing `GeodeParamsRunner`) pull request is introduced? I see there are quite a few failures in the CI `acceptance-test-openjdk11`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on pull request #6890: GEODE-4181: Migrate uses of JUnitParamsRunner
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on pull request #6890:
URL: https://github.com/apache/geode/pull/6890#issuecomment-926109338
> Great introduction of the pull request. Really appreciate that. Will this pull request introduce some temporary failures before another step 2 (re-implementing `GeodeParamsRunner`) pull request is introduced? I see there are quite a few failures in the CI `acceptance-test-openjdk11`.
I'll take a look at those failures. If any of those failures is due to this PR, that's a problem for me to fix. This PR is intended not to change test results in any way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal commented on pull request #6890: GEODE-4181: Migrate uses of JUnitParamsRunner
Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on pull request #6890:
URL: https://github.com/apache/geode/pull/6890#issuecomment-926126850
> Great introduction of the pull request. Really appreciate that. Will this pull request introduce some temporary failures before another step 2 (re-implementing `GeodeParamsRunner`) pull request is introduced? I see there are quite a few failures in the CI `acceptance-test-openjdk11`.
I'm confident that my PR did not cause the acceptance test failures, for these reasons:
- None of the failed tests use `GeodeParamsRunner` or `JUnitParamsRunner`, so none of them were _directly_ affected by my PR.
- Acceptance tests don't run in parallel, so my changes could not have interfered with the failed tests by inadvertently running some bogus test in parallel.
- The exact same tests failed in another PR precheckin (http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-6893/test-results/acceptanceTest/1632358535/), which does not include my changes.
- The "several gateway receivers" tests failed in another PR precheckin (http://files.apachegeode-ci.info/builds/apache-develop-pr/geode-pr-6891/test-results/acceptanceTest/1632349755/), which does not include my changes.
- My PR did not change which tests are executed. My PR executed the exact same acceptance tests as another PR (https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/acceptance-test-openjdk11/builds/1601), which does not include my changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] demery-pivotal merged pull request #6890: GEODE-4181: Migrate uses of JUnitParamsRunner
Posted by GitBox <gi...@apache.org>.
demery-pivotal merged pull request #6890:
URL: https://github.com/apache/geode/pull/6890
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org