You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2012/03/22 21:00:36 UTC
svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/
test-framework/src/java/org/apache/lucene/util/
Author: dweiss
Date: Thu Mar 22 20:00:35 2012
New Revision: 1304019
URL: http://svn.apache.org/viewvc?rev=1304019&view=rev
Log:
LUCENE-3847: ignore user.timezone because it is set by java logging system and this is hard to predict.
Modified:
lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSystemPropertiesInvariantRule.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesInvariantRule.java
lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesRestoreRule.java
Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSystemPropertiesInvariantRule.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSystemPropertiesInvariantRule.java?rev=1304019&r1=1304018&r2=1304019&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSystemPropertiesInvariantRule.java (original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSystemPropertiesInvariantRule.java Thu Mar 22 20:00:35 2012
@@ -20,11 +20,18 @@ package org.apache.lucene.util.junitcomp
import java.util.Properties;
import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.SystemPropertiesInvariantRule;
+import org.apache.lucene.util.SystemPropertiesRestoreRule;
import org.junit.*;
+import org.junit.rules.TestRule;
import org.junit.runner.JUnitCore;
import org.junit.runner.Result;
import org.junit.runner.notification.Failure;
+/**
+ * @see SystemPropertiesRestoreRule
+ * @see SystemPropertiesInvariantRule
+ */
public class TestSystemPropertiesInvariantRule extends WithNestedTests {
public static final String PROP_KEY1 = "new-property-1";
public static final String VALUE1 = "new-value-1";
@@ -85,6 +92,16 @@ public class TestSystemPropertiesInvaria
}
}
+ public static class IgnoredProperty {
+ @Rule
+ public TestRule invariant = new SystemPropertiesInvariantRule(PROP_KEY1);
+
+ @Test
+ public void testMethod1() {
+ System.setProperty(PROP_KEY1, VALUE1);
+ }
+ }
+
@Test
public void testRuleInvariantBeforeClass() {
Result runClasses = JUnitCore.runClasses(InBeforeClass.class);
@@ -120,4 +137,16 @@ public class TestSystemPropertiesInvaria
Assert.assertTrue(runClasses.getFailures().get(0).getMessage().contains("Will pass"));
Assert.assertEquals(3, runClasses.getRunCount());
}
+
+ @Test
+ public void testIgnoredProperty() {
+ System.clearProperty(PROP_KEY1);
+ try {
+ Result runClasses = JUnitCore.runClasses(IgnoredProperty.class);
+ Assert.assertEquals(0, runClasses.getFailureCount());
+ Assert.assertEquals(VALUE1, System.getProperty(PROP_KEY1));
+ } finally {
+ System.clearProperty(PROP_KEY1);
+ }
+ }
}
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java?rev=1304019&r1=1304018&r2=1304019&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java Thu Mar 22 20:00:35 2012
@@ -277,13 +277,23 @@ public abstract class LuceneTestCase ext
private static final UncaughtExceptionsRule uncaughtExceptionsRule = new UncaughtExceptionsRule(null);
/**
+ * These property keys will be ignored in verification of altered properties.
+ * @see SystemPropertiesInvariantRule
+ * @see #ruleChain
+ * @see #classRules
+ */
+ private static final String [] ignoredInvariantProperties = {
+ "user.timezone"
+ };
+
+ /**
* This controls how suite-level rules are nested. It is important that _all_ rules declared
* in {@link LuceneTestCase} are executed in proper order if they depend on each
* other.
*/
@ClassRule
public static TestRule classRules = RuleChain
- .outerRule(new SystemPropertiesInvariantRule())
+ .outerRule(new SystemPropertiesInvariantRule(ignoredInvariantProperties))
.around(classNameRule)
.around(uncaughtExceptionsRule);
@@ -297,7 +307,7 @@ public abstract class LuceneTestCase ext
.outerRule(new RememberThreadRule())
.around(new UncaughtExceptionsRule(this))
.around(new TestResultInterceptorRule())
- .around(new SystemPropertiesInvariantRule())
+ .around(new SystemPropertiesInvariantRule(ignoredInvariantProperties))
.around(new InternalSetupTeardownRule())
.around(new SubclassSetupTeardownRule());
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesInvariantRule.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesInvariantRule.java?rev=1304019&r1=1304018&r2=1304019&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesInvariantRule.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesInvariantRule.java Thu Mar 22 20:00:35 2012
@@ -18,7 +18,11 @@ package org.apache.lucene.util;
*/
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
import java.util.Iterator;
+import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
@@ -28,6 +32,32 @@ import org.junit.runners.model.MultipleF
import org.junit.runners.model.Statement;
public class SystemPropertiesInvariantRule implements TestRule {
+ /**
+ * Ignored property keys.
+ */
+ private final HashSet<String> ignoredProperties;
+
+ /**
+ * Cares about all properties.
+ */
+ public SystemPropertiesInvariantRule() {
+ this(Collections.<String>emptySet());
+ }
+
+ /**
+ * Don't care about the given set of properties.
+ */
+ public SystemPropertiesInvariantRule(String... ignoredProperties) {
+ this.ignoredProperties = new HashSet<String>(Arrays.asList(ignoredProperties));
+ }
+
+ /**
+ * Don't care about the given set of properties.
+ */
+ public SystemPropertiesInvariantRule(Set<String> ignoredProperties) {
+ this.ignoredProperties = new HashSet<String>(ignoredProperties);
+ }
+
@Override
public Statement apply(final Statement s, Description d) {
return new Statement() {
@@ -40,7 +70,12 @@ public class SystemPropertiesInvariantRu
} catch (Throwable t) {
errors.add(t);
} finally {
- TreeMap<String,String> after = SystemPropertiesRestoreRule.cloneAsMap(System.getProperties());
+ final TreeMap<String,String> after = SystemPropertiesRestoreRule.cloneAsMap(System.getProperties());
+
+ // Remove ignored if they exist.
+ before.keySet().removeAll(ignoredProperties);
+ after.keySet().removeAll(ignoredProperties);
+
if (!after.equals(before)) {
errors.add(
new AssertionError("System properties invariant violated.\n" +
@@ -48,7 +83,7 @@ public class SystemPropertiesInvariantRu
}
// Restore original properties.
- SystemPropertiesRestoreRule.restore(before, after);
+ SystemPropertiesRestoreRule.restore(before, after, ignoredProperties);
}
MultipleFailureException.assertEmpty(errors);
Modified: lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesRestoreRule.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesRestoreRule.java?rev=1304019&r1=1304018&r2=1304019&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesRestoreRule.java (original)
+++ lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/util/SystemPropertiesRestoreRule.java Thu Mar 22 20:00:35 2012
@@ -27,6 +27,32 @@ import org.junit.runners.model.Statement
* Restore system properties from before the nested {@link Statement}.
*/
public class SystemPropertiesRestoreRule implements TestRule {
+ /**
+ * Ignored property keys.
+ */
+ private final HashSet<String> ignoredProperties;
+
+ /**
+ * Restores all properties.
+ */
+ public SystemPropertiesRestoreRule() {
+ this(Collections.<String>emptySet());
+ }
+
+ /**
+ * @param ignoredProperties Properties that will be ignored (and will not be restored).
+ */
+ public SystemPropertiesRestoreRule(Set<String> ignoredProperties) {
+ this.ignoredProperties = new HashSet<String>(this.ignoredProperties);
+ }
+
+ /**
+ * @param ignoredProperties Properties that will be ignored (and will not be restored).
+ */
+ public SystemPropertiesRestoreRule(String... ignoredProperties) {
+ this.ignoredProperties = new HashSet<String>(Arrays.asList(ignoredProperties));
+ }
+
@Override
public Statement apply(final Statement s, Description d) {
return new Statement() {
@@ -39,7 +65,7 @@ public class SystemPropertiesRestoreRule
TreeMap<String,String> after = cloneAsMap(System.getProperties());
if (!after.equals(before)) {
// Restore original properties.
- restore(before, after);
+ restore(before, after, ignoredProperties);
}
}
}
@@ -69,16 +95,25 @@ public class SystemPropertiesRestoreRule
static void restore(
TreeMap<String,String> before,
- TreeMap<String,String> after) {
+ TreeMap<String,String> after,
+ Set<String> ignoredKeys) {
+
+ // Clear anything that is present after but wasn't before.
after.keySet().removeAll(before.keySet());
for (String key : after.keySet()) {
- System.clearProperty(key);
+ if (!ignoredKeys.contains(key))
+ System.clearProperty(key);
}
+
+ // Restore original property values unless they are ignored (then leave).
for (Map.Entry<String,String> e : before.entrySet()) {
- if (e.getValue() == null) {
- System.clearProperty(e.getKey()); // Can this happen?
- } else {
- System.setProperty(e.getKey(), e.getValue());
+ String key = e.getValue();
+ if (!ignoredKeys.contains(key)) {
+ if (key == null) {
+ System.clearProperty(e.getKey()); // Can this happen?
+ } else {
+ System.setProperty(e.getKey(), key);
+ }
}
}
}
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/
test-framework/src/java/org/apache/lucene/util/
Posted by Chris Hostetter <ho...@fucit.org>.
: This definitely should be cleaned up and I would love to break down
: the existing legacy hook methods into either rules or at least cleaner
: methods, but I'd rather do it after I land LUCENE-3808. I realize this
: sentence has become my usual defensive line recently.
that is not a defensive line, that is a "whet our appitites" line.
: So: you're right in that this isn't entirely like it should be
: (setting these "immutable" properties should be above sysprop
: invariants). Consider it a temporary workaround -- I'll try to cleanup
: LTC once that 3.x release is out.
No worries ... i was just curious if this was a slightly risky edge
situation, and it sounds like it is but you alrady have a plan to deal
with it eventually -- which is certianly good enough for me. (i'd only be
worried if you said it was a risk, and it's a big one, but we have no way
of dealing with it cleanly)
-Hoss
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Dawid Weiss <da...@cs.put.poznan.pl>.
Are there VMs other than (defunct?) Harmony that have a standard lib
other than SUN's/Oracle's? Can we check how others do it?
Dawid
On Thu, Mar 22, 2012 at 11:24 PM, Robert Muir <rc...@gmail.com> wrote:
> I guess they are allowed to do that, its not listed as a 'standard'
> sysprop in http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/System.html#getProperties%28%29
>
> But its just plain dirty
>
> On Thu, Mar 22, 2012 at 6:20 PM, Dawid Weiss
> <da...@cs.put.poznan.pl> wrote:
>> Yes, I was promising a beer to anybody who knew that before that
>> invariant was in place -- see my comment here:
>>
>> https://issues.apache.org/jira/browse/LUCENE-3847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13222013#comment-13222013
>>
>> Dawid
>>
>> On Thu, Mar 22, 2012 at 11:15 PM, Robert Muir <rc...@gmail.com> wrote:
>>> Its way worse than i thought:
>>>
>>> public static void main(String args[]) throws Exception {
>>> System.out.println("tz:" + System.getProperty("user.timezone"));
>>> TimeZone old = TimeZone.getDefault();
>>> System.out.println("tz:" + System.getProperty("user.timezone"));
>>> }
>>>
>>> prints:
>>>
>>> tz:
>>> tz:America/New_York
>>>
>>> So even calling TimeZone.getDefault() (and doing nothing with it) is
>>> enough to change the sysprop!
>>>
>>> On Thu, Mar 22, 2012 at 6:12 PM, Robert Muir <rc...@gmail.com> wrote:
>>>>>
>>>>> I didn't inspect in detail what TimeZone does in standard JDK (if it
>>>>> resets the property on each setDefault or only if it's empty?). Feel
>>>>> free to file an issue -- we may look into it in the future. I don't
>>>>> think this is that crucial (?).
>>>>>
>>>>
>>>> sorry i used locales where i should have used tzs too, but you get the idea :)
>>>> whether it resets or only if its empty, its the same problem for us
>>>> though, its something different than it was before (it was empty, now
>>>> it wont be)
>>>>
>>>>
>>>> --
>>>> lucidimagination.com
>>>
>>>
>>>
>>> --
>>> lucidimagination.com
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>>> For additional commands, e-mail: dev-help@lucene.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
>
>
>
> --
> lucidimagination.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
I guess they are allowed to do that, its not listed as a 'standard'
sysprop in http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/System.html#getProperties%28%29
But its just plain dirty
On Thu, Mar 22, 2012 at 6:20 PM, Dawid Weiss
<da...@cs.put.poznan.pl> wrote:
> Yes, I was promising a beer to anybody who knew that before that
> invariant was in place -- see my comment here:
>
> https://issues.apache.org/jira/browse/LUCENE-3847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13222013#comment-13222013
>
> Dawid
>
> On Thu, Mar 22, 2012 at 11:15 PM, Robert Muir <rc...@gmail.com> wrote:
>> Its way worse than i thought:
>>
>> public static void main(String args[]) throws Exception {
>> System.out.println("tz:" + System.getProperty("user.timezone"));
>> TimeZone old = TimeZone.getDefault();
>> System.out.println("tz:" + System.getProperty("user.timezone"));
>> }
>>
>> prints:
>>
>> tz:
>> tz:America/New_York
>>
>> So even calling TimeZone.getDefault() (and doing nothing with it) is
>> enough to change the sysprop!
>>
>> On Thu, Mar 22, 2012 at 6:12 PM, Robert Muir <rc...@gmail.com> wrote:
>>>>
>>>> I didn't inspect in detail what TimeZone does in standard JDK (if it
>>>> resets the property on each setDefault or only if it's empty?). Feel
>>>> free to file an issue -- we may look into it in the future. I don't
>>>> think this is that crucial (?).
>>>>
>>>
>>> sorry i used locales where i should have used tzs too, but you get the idea :)
>>> whether it resets or only if its empty, its the same problem for us
>>> though, its something different than it was before (it was empty, now
>>> it wont be)
>>>
>>>
>>> --
>>> lucidimagination.com
>>
>>
>>
>> --
>> lucidimagination.com
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Dawid Weiss <da...@cs.put.poznan.pl>.
Yes, I was promising a beer to anybody who knew that before that
invariant was in place -- see my comment here:
https://issues.apache.org/jira/browse/LUCENE-3847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13222013#comment-13222013
Dawid
On Thu, Mar 22, 2012 at 11:15 PM, Robert Muir <rc...@gmail.com> wrote:
> Its way worse than i thought:
>
> public static void main(String args[]) throws Exception {
> System.out.println("tz:" + System.getProperty("user.timezone"));
> TimeZone old = TimeZone.getDefault();
> System.out.println("tz:" + System.getProperty("user.timezone"));
> }
>
> prints:
>
> tz:
> tz:America/New_York
>
> So even calling TimeZone.getDefault() (and doing nothing with it) is
> enough to change the sysprop!
>
> On Thu, Mar 22, 2012 at 6:12 PM, Robert Muir <rc...@gmail.com> wrote:
>>>
>>> I didn't inspect in detail what TimeZone does in standard JDK (if it
>>> resets the property on each setDefault or only if it's empty?). Feel
>>> free to file an issue -- we may look into it in the future. I don't
>>> think this is that crucial (?).
>>>
>>
>> sorry i used locales where i should have used tzs too, but you get the idea :)
>> whether it resets or only if its empty, its the same problem for us
>> though, its something different than it was before (it was empty, now
>> it wont be)
>>
>>
>> --
>> lucidimagination.com
>
>
>
> --
> lucidimagination.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
Its way worse than i thought:
public static void main(String args[]) throws Exception {
System.out.println("tz:" + System.getProperty("user.timezone"));
TimeZone old = TimeZone.getDefault();
System.out.println("tz:" + System.getProperty("user.timezone"));
}
prints:
tz:
tz:America/New_York
So even calling TimeZone.getDefault() (and doing nothing with it) is
enough to change the sysprop!
On Thu, Mar 22, 2012 at 6:12 PM, Robert Muir <rc...@gmail.com> wrote:
>>
>> I didn't inspect in detail what TimeZone does in standard JDK (if it
>> resets the property on each setDefault or only if it's empty?). Feel
>> free to file an issue -- we may look into it in the future. I don't
>> think this is that crucial (?).
>>
>
> sorry i used locales where i should have used tzs too, but you get the idea :)
> whether it resets or only if its empty, its the same problem for us
> though, its something different than it was before (it was empty, now
> it wont be)
>
>
> --
> lucidimagination.com
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
>
> I didn't inspect in detail what TimeZone does in standard JDK (if it
> resets the property on each setDefault or only if it's empty?). Feel
> free to file an issue -- we may look into it in the future. I don't
> think this is that crucial (?).
>
sorry i used locales where i should have used tzs too, but you get the idea :)
whether it resets or only if its empty, its the same problem for us
though, its something different than it was before (it was empty, now
it wont be)
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Dawid Weiss <da...@cs.put.poznan.pl>.
> // user.timezone is empty
> TimeZone old = TimeZone.getDefault(); // lets say en-US
> TimeZone.setDefault(something); // lets say en-UK
> ...
>
> TimeZone.setDefault(old); // restore the default en-US
>
> // now user.timezone is en-US (changed from nothing) as a side effect,
> even though we restored what we did
I didn't inspect in detail what TimeZone does in standard JDK (if it
resets the property on each setDefault or only if it's empty?). Feel
free to file an issue -- we may look into it in the future. I don't
think this is that crucial (?).
Dawid
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
yeah, from my understanding, the problem is if i do this:
// user.timezone is empty
TimeZone old = TimeZone.getDefault(); // lets say en-US
TimeZone.setDefault(something); // lets say en-UK
...
TimeZone.setDefault(old); // restore the default en-US
// now user.timezone is en-US (changed from nothing) as a side effect,
even though we restored what we did
On Thu, Mar 22, 2012 at 5:59 PM, Dawid Weiss
<da...@cs.put.poznan.pl> wrote:
>> that begs the question: why then ddoes david need to treat it as special
>> at all? shouldn't it be set at the begining by the randomization code and
>
> It's because of how rules are nested, really. Class rules are "around"
> any @BeforeClass/@AfterClass hooks (and @Before/@After hooks) so
> hooks like LuceneTestCase#beforeClassLuceneTestCaseJ4 are called after
> the invariant rule recorded the set of properties. We do reset this
> particular property to its value seen before (#restoreProperties) but
> occasionally another class will have a class or test hook
> (@BeforeClass) that will trigger property change via logging or
> something else.
>
> This definitely should be cleaned up and I would love to break down
> the existing legacy hook methods into either rules or at least cleaner
> methods, but I'd rather do it after I land LUCENE-3808. I realize this
> sentence has become my usual defensive line recently.
>
> So: you're right in that this isn't entirely like it should be
> (setting these "immutable" properties should be above sysprop
> invariants). Consider it a temporary workaround -- I'll try to cleanup
> LTC once that 3.x release is out.
>
> Dawid
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Dawid Weiss <da...@cs.put.poznan.pl>.
> that begs the question: why then ddoes david need to treat it as special
> at all? shouldn't it be set at the begining by the randomization code and
It's because of how rules are nested, really. Class rules are "around"
any @BeforeClass/@AfterClass hooks (and @Before/@After hooks) so
hooks like LuceneTestCase#beforeClassLuceneTestCaseJ4 are called after
the invariant rule recorded the set of properties. We do reset this
particular property to its value seen before (#restoreProperties) but
occasionally another class will have a class or test hook
(@BeforeClass) that will trigger property change via logging or
something else.
This definitely should be cleaned up and I would love to break down
the existing legacy hook methods into either rules or at least cleaner
methods, but I'd rather do it after I land LUCENE-3808. I realize this
sentence has become my usual defensive line recently.
So: you're right in that this isn't entirely like it should be
(setting these "immutable" properties should be above sysprop
invariants). Consider it a temporary workaround -- I'll try to cleanup
LTC once that 3.x release is out.
Dawid
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/
test-framework/src/java/org/apache/lucene/util/
Posted by Chris Hostetter <ho...@fucit.org>.
: We already don't have to worry about this: locale and timezone are set
: statically for the whole test class.
:
: But by randomizing it, we also don't have to worry about improper use
right, of course ... i forgot we were explicitly randomizing it -- but
that begs the question: why then ddoes david need to treat it as special
at all? shouldn't it be set at the begining by the randomization code and
then never changed?
is that hapening *after* the property rules are recording what the
starting system properties are? should it be happening *before*? (not just
for timezone, i'm wondering if there is the possibility of other risks
/ edge cases we may be overlooking)
-Hoss
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
On Thu, Mar 22, 2012 at 4:08 PM, Chris Hostetter
<ho...@fucit.org> wrote:
> (that way, when interpreting test logs, we never have to worry
> that some DST shift happened during the tests that might be
> making the timestamps confusing)
>
We already don't have to worry about this: locale and timezone are set
statically for the whole test class.
But by randomizing it, we also don't have to worry about improper use
of the default tz/locale, which pops up from time to time, especially
turkish :)
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
Here's the history of this: https://issues.apache.org/jira/browse/SOLR-1821
Great example of a test that only failed for some developers in one
continent but not others.
By randomizing things like this, we know that the stuff actually works
for users in all possible tzs/locales, and it also reproduces for
developers regardless of where they are.
On Thu, Mar 22, 2012 at 4:18 PM, Dawid Weiss
<da...@cs.put.poznan.pl> wrote:
>> crazy thought: what if instead of ignoring it in these checks, we force
>> set it to UTC? either in ant or in the test runner .. whichever makes
>
> We actually randomize TimeZone (and this property as a side effect) in
> LuceneTestCase... I didn't write that though so I don't know if
> removing it is a good idea. It definitely adds fun to watch logs
> emitted in different time formats when the locale/ time zone is
> randomized...
>
> Dawid
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Robert Muir <rc...@gmail.com>.
I added this because it finds bugs where the default timezone and
locale are found.
We cannot set the time to UTC, because most users don't do this
either. The default locale and timezone can be anything really.
On Thu, Mar 22, 2012 at 4:18 PM, Dawid Weiss
<da...@cs.put.poznan.pl> wrote:
>> crazy thought: what if instead of ignoring it in these checks, we force
>> set it to UTC? either in ant or in the test runner .. whichever makes
>
> We actually randomize TimeZone (and this property as a side effect) in
> LuceneTestCase... I didn't write that though so I don't know if
> removing it is a good idea. It definitely adds fun to watch logs
> emitted in different time formats when the locale/ time zone is
> randomized...
>
> Dawid
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
--
lucidimagination.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/ test-framework/src/java/org/apache/lucene/util/
Posted by Dawid Weiss <da...@cs.put.poznan.pl>.
> crazy thought: what if instead of ignoring it in these checks, we force
> set it to UTC? either in ant or in the test runner .. whichever makes
We actually randomize TimeZone (and this property as a side effect) in
LuceneTestCase... I didn't write that though so I don't know if
removing it is a good idea. It definitely adds fun to watch logs
emitted in different time formats when the locale/ time zone is
randomized...
Dawid
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: svn commit: r1304019 - in /lucene/dev/trunk/lucene:
core/src/test/org/apache/lucene/util/junitcompat/
test-framework/src/java/org/apache/lucene/util/
Posted by Chris Hostetter <ho...@fucit.org>.
: LUCENE-3847: ignore user.timezone because it is set by java logging system and this is hard to predict.
crazy thought: what if instead of ignoring it in these checks, we force
set it to UTC? either in ant or in the test runner .. whichever makes
more sense. (that way, when interpreting test logs, we never have to worry
that some DST shift happened during the tests that might be
making the timestamps confusing)
-Hoss
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org