On 2014-01-07 15:45, Alan Bateman wrote:
On 07/01/2014 14:29, Erik Joelsson wrote:
Updated review with a regression test. I'm not familiar with jtreg so
tried pattern matching on existing tests and translated the
reproducer from the bug report. I suspect that this test should not
be run in the same jvm instance as other tests as it destructively
changes the system properties. Is that something that can be
expressed to jtreg?
http://cr.openjdk.java.net/~erikj/8030781/webrev.01/
/Erik
Thanks for spending time on test, I wasn't sure if this was going to
be a separate JBS issue or not.
I think a more complete test would check that the other system
properties are reset (as in the table in System#getProperties). Also I
think it should change some of the these to nonsense values and see
that setProperties(null) restores them to their original values. This
might be a bit more than what you were thinking but it's the sort of
test that should have been there to catch this issue in the first place.
So, do you think I should:
1. Commit the fix without the test and file a followup for a proper test
to be implemented.
2. Commit the fix with the current test and file a followup for making
the test better.
3. Fix the test first and then commit the fix.
3 is certainly best, but I rather don't have the time for it right now.
Which of 1 or 2 is preferred?
"@run main/othervm SetPropertiesNull" will ensure that it runs in its
own VM but it might not be necessary here as system properties is one
of the things that jtreg will reset.
That should cover us, except that if this test fails, so will jtreg fail
with resetting the properties.
/Erik
