-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Adds fallback dictionary when serialising device info. #279
Changes from 3 commits
2a4844e
9060518
7dbe713
2fb139d
c2bf343
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// | ||
// BugsnagKSCrashSysInfoParserTestTest.m | ||
// Tests | ||
// | ||
// Created by Jamie Lynch on 15/05/2018. | ||
// Copyright © 2018 Bugsnag. All rights reserved. | ||
// | ||
|
||
#import <XCTest/XCTest.h> | ||
|
||
#import "BugsnagKSCrashSysInfoParser.h" | ||
|
||
@interface BugsnagKSCrashSysInfoParserTest : XCTestCase | ||
@end | ||
|
||
@implementation BugsnagKSCrashSysInfoParserTest | ||
|
||
- (void)testEmptyDictSerialisation { | ||
// ensures that an empty dictionary parameter returns a fallback dictionary populated with at least some information | ||
NSDictionary *device = BSGParseDevice(@{}); | ||
XCTAssertNotNil(device); | ||
XCTAssertNotNil(device[@"locale"]); | ||
XCTAssertNotNil(device[@"freeDisk"]); | ||
XCTAssertNotNil(device[@"simulator"]); | ||
} | ||
|
||
@end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the report argument is nil? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tested this out and a dictionary is returned with the same contents as if an empty dictionary had been passed in. Adding this as a test would currently fail the suite with an error, presumably due to the fact we've enabled strict build settings and this method parameter is enabled as NonNull. Do you feel we need to change anything so that this can be unit tested? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to loosen the restrictions on just the test files? It seems like it might be useful to have a test like that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a test for a valid dictionary that contains the expected key? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is tested elsewhere I believe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to do something like this? I'm not sure how
initWithDictionary
handles nilThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initWithDictionary:
requires a Nonnull parameter, whereasaddEntriesFromDictionary:
handles a nil param.