boredzo / iso-8601-date-formatter

A Cocoa NSFormatter subclass to convert dates to and from ISO-8601-formatted strings. Supports calendar, week, and ordinal formats.
http://boredzo.org/iso8601dateformatter/
Other
600 stars 140 forks source link

Odd output from date formatter (including AM/PM markers?) #15

Closed tonymillion closed 10 years ago

tonymillion commented 11 years ago

I'm still investigating the whys and wherefores (as I only saw this in a production app calling the API) but we've seen output like this occasionally:

2013-08-02T01:35:00 PMZ

It looks like its formatting the time as 01:30:00 PM then adding the 'Z' zone?

I'll update this ticket as we find out more information the code that produces the string is this:

    ISO8601DateFormatter *fmt = [[ISO8601DateFormatter alloc] init];
    NSTimeZone * tz = [NSTimeZone localTimeZone];
    fmt.includeTime = YES;

    if(self.timezone)
    {
        tz = [NSTimeZone timeZoneWithName:self.timezone];
    }

    return [fmt stringFromDate:self.created
                      timeZone:tz];

In the error seen above self.timezone was @"GMT" (which is what cocoa calls UTC) I don't know anything about the locale of the specific device but I believe that could have affected this.

tonymillion commented 11 years ago

Yep now that I'm looking harder I found this:

2013-08-17T09:16:47 du.+0200

There is something horribly wrong.

boredzo commented 11 years ago

What time zone was tz ([NSTimeZone localTimeZone])?

tonymillion commented 11 years ago

Yes it would have been localTimeZone - the du. response was coming from the Europe/Budapest I believe. Unfortunately I can't say anything about the specific settings under settings->general->International which can affect NSDateFormatters.

see this for more info (I think this could be the cause, and it something we'd test on the app): http://stackoverflow.com/questions/6613110/what-is-the-best-way-to-deal-with-the-nsdateformatter-locale-feature

boredzo commented 11 years ago

Apparently this behavior is iOS-specific, which is why none of my test cases were catching it: https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/DataFormatting/Articles/dfDateFormatting10_4.html#//apple_ref/doc/uid/TP40002369-SW13

boredzo commented 11 years ago

Inconveniently, I still can't repro the bug. Does the test case fail for you (when using the Cocoa Touch test bundle)?

mattbauch commented 11 years ago

It'll be a bit hard to test for this bug because as far as I know enabling the forced 12 hour mode is not exposed by any API.

But NSLocale has a private method _prefs which will return a dictionary with locale information about the system.

If the system is configured to a locale that naturally uses 24 hour mode and you force iOS to display the time in 12 hour mode, there will be true boolean for the key AppleICUForce12HourTime in the _prefs dict. Something similar happens if you force 24 hour mode on a 12 hour locale, but the key will be named AppleICUForce24HourTime.

In theory, if you want to write a test that is independent of the systems locale you have to return a _prefs dictionary that has a true boolean for the key AppleICUForce12HourTime.

So I created a "mock" object for NSLocale. And it actually works, even with the simulator. Looks like the simulator just doesn't expose the Settings UI to activate this mode.

@interface MBLocale : NSObject <NSCopying>
@end

@implementation MBLocale {
    NSLocale *locale;
}

- (id)init {
    self = [super init];
    locale = [[NSLocale alloc] initWithLocaleIdentifier:@"de_DE"];
    return self;
}

- (id)copyWithZone:(NSZone *)zone {
    return [[[self class] allocWithZone:zone] init];
}

- (NSString *)localeIdentifier {
    return [locale localeIdentifier];
}

- (id)objectForKey:(NSString *)key {
    id object = [locale objectForKey:key];
    return object;
}

- (NSDictionary *)_prefs {
    NSMutableDictionary *prefs = [NSMutableDictionary dictionaryWithDictionary:[locale performSelector:@selector(_prefs)]];
    [prefs setObject:@YES forKey:@"AppleICUForce12HourTime"];
    return prefs;
}
@end

The following snippet logs 02:48:41 vorm. (vorm. = AM):

NSDateFormatter *df = [[NSDateFormatter alloc] init];
df.locale = (NSLocale *)[[MBLocale alloc] init];
df.dateFormat = @"HH:mm:ss";
NSLog(@"%@", [df stringFromDate:[NSDate date]]);

Looks good so far. Now you just have to use that class in ISO8601DateFormatter.

I don't know if method swizzling is acceptable for your unit tests, but if you swizzle +[NSLocale currentLocale] with a method that returns the MBLocale mock object, -[ISO8601DateFormatter stringFromDate:] returns the wrong formatted date.

yay.
But since I'm too stupid to grok your test code setup I'm unable to put this into a pull request. ;-) And maybe this whole mess is too complicated and there is an easier way.


I'm not sure if I interpret your last comment correctly, but it should be quite easy to reproduce the problem on a device.

In iOS settings General/International/Region Format set the region to German/Germany. In General/Date&Time switch 24-Hour time to off.

boredzo commented 11 years ago

Thank you, @mattbauch! I'll give that a shot; it looks very promising.

I'm honestly not interested in making any test require running on a device. Everything should be testable on a Mac alone.

There are two test setups: the old pre-OCTest test monsters (involving shell, Python, and ultimately command-line tools that generate text files of output) and the newer OCTest bundle targets. The Makefile runs the test monsters and the Xcode project runs the OCTest tests. All new tests must be OCTest-based; the test monsters will be incinerated as soon as they're fully redundant.

OCTest is pretty straightforward. Every individual test case is one test method in a test case class. The date formatter is created by setUp and destroyed by tearDown, which are called before and after each test method. The test method actually tests things by calling STAssert…(…); if the assertion is false, the test fails.

mattbauch commented 11 years ago

Okay, I was just confused about make test and thought they would somehow run the OCTests too. I added the mock object and the method swizzling to the test and it seems to work.

boredzo commented 11 years ago

make test probably should run the OCUnit tests after the test monsters are slain, but I'm not anywhere near that point yet.

boredzo commented 11 years ago

Thank you, @mattbauch.

Incidentally, you may want to adapt your earlier explanation to an answer on the Stack Overflow question I asked about testing this. (You can leave out the code—it would be kind of a long answer with that in it.)

boredzo commented 10 years ago

According to Wil Shipley, this happens on Mavericks, too: https://twitter.com/wilshipley/status/435387956862283776

I'll look into this tonight.

boredzo commented 10 years ago

Confirmed that I can reproduce this with the existing test case on Mavericks. The existing fix works on Mavericks, too.