Rethink LocalizedTestCaseRunner with JUnit 4 - Clover OOM [LUCENE-2652]

[asfimport]

As a spinn off from this conversation we should rethink the way how we execute testcases with different locals since glover reports appears to throw OOM errors b/c Junit treats each local as a single test case run.

Here are some options:

• select the local at random only run the test with a single local
• set the local via system property -Dtest.locale=en.EN
• run with the default locale only -Dtest.skiplocale=true
• one from the above but only if instrumented with clover (let common tests run all the locale)

---

Migrated from LUCENE-2652 by Simon Willnauer (@s1monw), resolved Oct 19 2010 Attachments: LUCENE-2652.patch (versions: 4)

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

here's a patch to disable use of the runners.

really i think the two runners (LocalizedTestCase/MultiCodecTestCase) are obselete anyway, you can run any junit test under any locale/codec, and we select locale, timezone, and codec randomly by default.

[asfimport]

Simon Willnauer (@s1monw) (migrated from JIRA)

really i think the two runners (LocalizedTestCase/MultiCodecTestCase) are obselete anyway, you can run any junit test under any locale/codec, and we select locale, timezone, and codec randomly by default.

Yeah I agree that we should fix that and get rid of them. I also think we should apply a quick fix to make builds on hudson pass again. Yet, you patch changes lots of files - the patch I attached is less intrusive and has the same effect.

Lets commit that and do a real fix within the next days. Thoughts?

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Yet, you patch changes lots of files

Right, but long term we should remove these runners anyway. So maybe we should delete the RunWiths and delete the runner code.

These runners are obselete and weaker than our general test support itself:

• the MultiCodecRunner cannot set parameters for the codecs, but LuceneTestCase can do this
• The LocalizedRunner cannot set timezone (this runner is used by lots of tests), but LuceneTestCase can do this

[asfimport]

Simon Willnauer (@s1monw) (migrated from JIRA)

Right, but long term we should remove these runners anyway. So maybe we should delete the RunWiths and delete the runner code.

I agree robert but I want to let other folk have a chance to comment on how to proceed (its weekend and some may have other things to do) but at the same time I don't want to let hudson fail again due to clover / Junit 4.

I'd commit the fix to trunk and 3.x and decide once others have commented, thoughts?

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

I'd commit the fix to trunk and 3.x and decide once others have commented, thoughts?

+1, commit it so our clover works

[asfimport]

Simon Willnauer (@s1monw) (migrated from JIRA)

I committed the quick fix for now

• 3.x – Committed revision 998503.
• trunk – Committed revision 998507.

I will work on a new patch tomorrow.

simon

[asfimport]

Simon Willnauer (@s1monw) (migrated from JIRA)

I added support for tests.locale=all && tests.codec=all while still utilizing the TestRunners we used before. With this patch we preserve all functionality we had before - being able to run with all locale / codec types for those tests which have been marked with the corresponding runners. Should we commit that until we change to a single runner?

Yet, i think what we should ultimately do is remove the runners and add support to run all permutations like a test with all codecs, locale , timezone etc. I don't think that it makes lot of sense to do that for every test but maybe we want to offer the test to tell if it wants to run with all locale and / or all codec ....

maybe something like

enum TestProperty {
  Locale, Codec, Timezone

}

...

setTestProperties(TestProperty ... properties) {
  ...
}

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

I added support for tests.locale=all && tests.codec=all while still utilizing the TestRunners we used before. With this patch we preserve all functionality we had before - being able to run with all locale / codec types for those tests which have been marked with the corresponding runners. Should we commit that until we change to a single runner?

Not sure, i think its a little confusing, because this 'all' only affects tests that use the runner.

I still think we should remove these runners completely. there is other ways to test all locales/codecs: for example by doing ant test -Dtestcase=foo -Dtests.iter=1000, as it should randomly hit them all anyway, same with codecs.

and this works for all test cases, not just those annotated with the runners.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Rethinking: Now that all tests are Junit4 tests, this issue is obselete.

these runners don't properly setup the locale correctly: only in setUp+test method itself.

But our build system now with -Dtests.locale does it correctly, @beforeClass is consistent too.

I'll remove these runners from LuceneTestCase shortly.

[asfimport]

Simon Willnauer (@s1monw) (migrated from JIRA)

I'll remove these runners from LuceneTestCase shortly.

+1 thanks for taking this issue!

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Committed revision 1024219, 1024223 (3x)

[asfimport]

Grant Ingersoll (@gsingers) (migrated from JIRA)

Bulk close for 3.1