Use JUnit4 capabilites for more thorough Locale testing for classes deriving from LocalizedTestCase [LUCENE-2122]

[asfimport]

Use the @Parameterized capabilities of Junit4 to allow more extensive testing of Locales.

---

Migrated from LUCENE-2122 by Erick Erickson (@ErickErickson), updated May 09 2016
Attachments: LUCENE-2122.patch (versions: 2), LUCENE-2122-r2.patch, LUCENE-2122-r3.patch, LUCENE-2122-r4.patch

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

All tests pass. This modifies all test classes (core and contrib) that derive from LocalizedTestCase. LocalizedTestCase now tests all test methods in all derived classes against all available Locales.

If we want some of the tests to NOT run against all locales, we'd need to refactor them into their own test class....

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Hi Erick, I am a little nervous about the change to LocalizedTestCase.tearDown() here.

I think we must restore the users default Locale, since its a JRE-system wide global thing and we are changing it on the fly here.

this was stashed away here before:

 /**
   * Before changing the default Locale, save the default Locale here so that it
   * can be restored.
   */
  private final Locale defaultLocale = Locale.getDefault();

and restored in tearDown()... otherwise strange things could happen, such as your IDE could go bonkers after running the tests! (but maybe I am missing something)

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

Restoring original default Locale after test class has been run.

Thanks Robert!

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Erick do you think LocalizedTestCase should be abstract?

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

Made LocalizedTestCase abstract...

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Hi Erick, in the Date tools test I think you can delete the public static Collection<Locale[]> data(), I think you might have accidentally included it?

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

OK, I plead advanced senility or some other excuse for the last patch.

Robert:
Thanks so much for looking this over, I have no clue what I was thinking with the TestDateTools. Or the other classes that derive from LocalizedTestCase.

The @Parameterized and @RunWith only needed to be in LocalizedTestCase and all the inheriting classes just rely on the base class to collect the different locales.

Anyway, this one should be much better....

Erick

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

thanks Erick, i will play around with the patch some, generally just double-check the locale stuff is doing what we want, looks like it will.

i havent tested yet, but looking at the code i have a few questions (i can try to add these to the patch just curious what you think):

1. if a test fails under some locale, say th_TH, will junit 4 attempt to print this parameter out in some way so I know that it failed? If not do you know of a hack?
2. i am thinking about reordering the locale array so that it tests the default one first. if you are trying to do some test-driven dev it might be strange if the test fails under a different locale first. I think this one is obvious, I will play with it to see how it behaves now.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Hi Erick, I played with this patch some and (not intentionally trying) I would get random test failures for TestQueryParser under eclipse... its not really something I am able to repeat though.

maybe some race condition (I do not know how eclipse executes parameterized tests).... ?

if it is a problem with my IDE that is one thing, just makes me a little nervous right now. trying to think what could cause this....

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Hi Erick, I spent some time with this patch today and here is what happened:

1. i tried to simplify localizedtestcase, so that it works just like the old one, with the exception of using junit4 parameterized facility.
2. i wrote some bad tests and ensured things worked well such as error messages, default locale being run first, etc etc.
3. i had everything good to go when i got random failures again, this time from 'ant clean test' about 3 times (pass,fail,pass)
   (sorry i should have done something to capture each test log but i did not)

because the only real change here is use of the parameterized facility (the logic is the same), it makes me think that we should stick with .runBare() for the time being, because there is something strange going on here and I'm not even trying to break it.

attached is the modified version of your patch.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

i am unassigning in case someone else can figure this one out, at my wits end here :)
perhaps its just something wierd about my environment or something

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

btw, I left 'ant clean test' running in a loop and just checked it with this patch, no problems.
so perhaps its my own incompetence. Erick can you take a look? Do you see some obvious problem?

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

Robert:

Where are we on this? Last I knew your test problems had disappeared, but memory can be tricky....

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Erick the test problems havent gone away (see latest hudson failure), but as you can see they appear to be unrelated to your patch, but existing problems.

I am glad Uwe and Hudson are now seeing the failures I saw, but unfortunately I am unable to reproduce this problem. I think it is very wierd that it only recently has started happening and the Junit 4 jar file is looking very suspicious to me :)

The next step is to force another test failure (with existing trunk code), except with an additional assertion in TestQueryParser.getParser(), that asserts that the created queryparser has the same Locale as LocalizedTestcase.locale

If this assertion is not triggered, and a failure happens anyway, then there is no problem with LocalizedTestCase and we should commit your patch. This would mean the problem is instead some problem in the query parser, its tests, date calculations, jvm handling of thai dates, something else, but not the locale-switching.

if the assertion is triggered, then we need to figure how why before changing the locale-swapping mechanism.

I ran this test 2500 times last night but i couldnt make it fail. i have a very small portable right now and I will need to wait till the new year to really look at this more...

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Some additional infos about the Hudson failure:
In the past Hudson used an internal junit.jar to do the testing, which was in version 4 (the exact version can only be found out by one with hudson access, Mike?). So Hudson was always using Junit4 for testing even in Lucene 2.9. After the clover updates, we removed this JUnit lib path from the ANT command line and therefore also hudson uses 4.7 as shipped with the svn checkout.

If it is really a JUnit problem, it seems to only occur with version 4.7. But I am not sure, I still stink it may be a thread starvation problem (slow test) or some local time problem that occurred two nights ago.