[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533284#comment-14533284 ] ASF GitHub Bot commented on LANG-1127: -- Github user coveralls commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-5716 [](https://coveralls.io/builds/2512304) Coverage decreased (-0.04%) to 93.27% when pulling **d68f7f5f44924ddee14428e16b7458e2878baef2 on britter:LANG-1127** into **8e7ea70a33ecb8db5207849ea6371b4a5a8ffd26 on apache:master**. Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Discussion We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533300#comment-14533300 ] ASF GitHub Bot commented on LANG-1127: -- Github user chonton commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-12112 Iterate fast and often. Go forward with it. Chas On May 7, 2015, at 12:17 PM, Benedikt Ritter notificati...@github.com wrote: @chonton I've worked some more on this. Renamed the Rule to SystemDefaultsSwitch and wrote a unit test. What do you think about integrating this now and create a new PR for the synchronization? ― Reply to this email directly or view it on GitHub. Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Discussion We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14533230#comment-14533230 ] ASF GitHub Bot commented on LANG-1127: -- Github user britter commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-99986739 @chonton I've worked some more on this. Renamed the Rule to SystemDefaultsSwitch and wrote a unit test. What do you think about integrating this now and create a new PR for the synchronization? Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Discussion We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14529818#comment-14529818 ] Charles Honton commented on LANG-1127: -- Yes, as the default system values change, the cache will fill up with additional formats. However, the key to the cache is always some actual timezone/locale pair (not nulls). The proper format will be returned irregardless of the current system defaults. I believe there is even a unit test that covers this case. Chas Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14529957#comment-14529957 ] ASF GitHub Bot commented on LANG-1127: -- Github user coveralls commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-99328066 [](https://coveralls.io/builds/2496885) Coverage increased (+0.06%) to 93.37% when pulling **3dbf1eedd93565481f8b5d3c9678a5e9234cb91a on britter:LANG-1127** into **8e7ea70a33ecb8db5207849ea6371b4a5a8ffd26 on apache:master**. Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14526842#comment-14526842 ] ASF GitHub Bot commented on LANG-1127: -- Github user chonton commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-9878 Overall, I like this approach better than explicit classes. Two items I would like to see: 1. Allow tests to run in parallel. This will probably requires some sort of synchronization to allow only one tests at a time to change default TimeZone or Locale. 2. Apply the rule to individual test methods instead of to all methods in a test class. Perhaps this could be done by creating annotation(s) which enable the Rule. I am thinking of something like SwitchDefaults(locale=locale_name, timezone=timezone_name). Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14526987#comment-14526987 ] ASF GitHub Bot commented on LANG-1127: -- Github user britter commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-98803478 Hello @chonton, very good feedback, thank you! 1. Allow tests to run in parallel. This will probably requires some sort of synchronization to allow only one tests at a time to change default TimeZone or Locale. Good point. Should be possible by wrapping the code in the apply method with some synchronization logic. 2. Apply the rule to individual test methods instead of to all methods in a test class. Perhaps this could be done by creating annotation(s) which enable the Rule. I am thinking of something like SwitchDefaults(locale=locale_name, timezone=timezone_name). This is already possible by calling explicitly the setLocale method on the TestLocale instance. The idea is to set up a default for all tests and only switch it for tests when needed. However this doesn't make much sense in the light of parallel test execution... Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[
https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14525791#comment-14525791
]
Christian P. MOMON commented on LANG-1127:
--
Hello, I support the goal of this ticket. I want to draw your attention to a
technical element that seems important to take into account.
{noformat}
Create a base test for the time package, which sets and resets default
Locales and TimeZones
Key: LANG-1127
URL: https://issues.apache.org/jira/browse/LANG-1127
Project: Commons Lang
Issue Type: Test
Components: lang.time.*
Reporter: Benedikt Ritter
Assignee: Charles Honton
Fix For: Patch Needed
We have a lot of problems due to Locale dependent tests. I propose to create
a base test class with a setup and teardown method which set and reset
default locale and time zone. All other tests should inherit from this base
test class so that they don't have to fiddle around with default settings.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[
https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14525802#comment-14525802
]
Christian P. MOMON commented on LANG-1127:
--
Hello, I support the goal of this ticket. I want to draw your attention to a
technical element that seems important to take into account. (!)
Many methods from time package are using the FormatCache class. Example:
{noformat}
class FastDateFormat {
[...]
public static FastDateFormat getInstance(final String pattern, final TimeZone
timeZone) {
return cache.getInstance(pattern, timeZone, null);
}
[...]
{noformat}
FormatCache.java :
{noformat}
public F getInstance(final String pattern, TimeZone timeZone, Locale
locale) {
if (pattern == null) {
throw new NullPointerException(pattern must not be null);
}
if (timeZone == null) {
timeZone = TimeZone.getDefault();
}
if (locale == null) {
locale = Locale.getDefault();
}
final MultipartKey key = new MultipartKey(pattern, timeZone, locale);
F format = cInstanceCache.get(key);
if (format == null) {
format = createInstance(pattern, timeZone, locale);
final F previousValue= cInstanceCache.putIfAbsent(key, format);
if (previousValue != null) {
// another thread snuck in and did the same work
// we should return the instance that is in ConcurrentMap
format= previousValue;
}
}
return format;
}
{noformat}
So, unit tests will fill the FormatCache object with some default system
values. If you change the default system value then the old cache values are in
it again. (!)
Change the default system timezone requires to reset the FormatCache objet. (+)
I suggest to add a reset method in FormatCache class. (on)
Hope this will help you. :)
Create a base test for the time package, which sets and resets default
Locales and TimeZones
Key: LANG-1127
URL: https://issues.apache.org/jira/browse/LANG-1127
Project: Commons Lang
Issue Type: Test
Components: lang.time.*
Reporter: Benedikt Ritter
Assignee: Charles Honton
Fix For: Patch Needed
We have a lot of problems due to Locale dependent tests. I propose to create
a base test class with a setup and teardown method which set and reset
default locale and time zone. All other tests should inherit from this base
test class so that they don't have to fiddle around with default settings.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14525811#comment-14525811 ] ASF GitHub Bot commented on LANG-1127: -- Github user coveralls commented on the pull request: https://github.com/apache/commons-lang/pull/76#issuecomment-98475017 [](https://coveralls.io/builds/2475426) Coverage decreased (-0.01%) to 93.3% when pulling **abfcf0a8cfb40cd30c83d30923bea6d412cc5a22 on britter:LANG-1127** into **8e7ea70a33ecb8db5207849ea6371b4a5a8ffd26 on apache:master**. Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14525807#comment-14525807 ] ASF GitHub Bot commented on LANG-1127: -- GitHub user britter opened a pull request: https://github.com/apache/commons-lang/pull/76 LANG-1127: Use JUnit rules to set and reset the default Locale and TimeZone Alternative proposal to fix [LANG-1127](https://issues.apache.org/jira/browse/LANG-1127) using JUnit TestRules. You can merge this pull request into a Git repository by running: $ git pull https://github.com/britter/commons-lang LANG-1127 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/76.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #76 commit abfcf0a8cfb40cd30c83d30923bea6d412cc5a22 Author: Benedikt Ritter brit...@apache.org Date: 2015-05-03T12:20:03Z LANG-1127: Use JUnit rules to set and reset the default Locale and TimeZone. Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (LANG-1127) Create a base test for the time package, which sets and resets default Locales and TimeZones
[ https://issues.apache.org/jira/browse/LANG-1127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14525809#comment-14525809 ] Benedikt Ritter commented on LANG-1127: --- Here is my proposal for fixing this: https://github.com/apache/commons-lang/pull/76 please review. Create a base test for the time package, which sets and resets default Locales and TimeZones Key: LANG-1127 URL: https://issues.apache.org/jira/browse/LANG-1127 Project: Commons Lang Issue Type: Test Components: lang.time.* Reporter: Benedikt Ritter Assignee: Charles Honton Fix For: Patch Needed We have a lot of problems due to Locale dependent tests. I propose to create a base test class with a setup and teardown method which set and reset default locale and time zone. All other tests should inherit from this base test class so that they don't have to fiddle around with default settings. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
