-
Notifications
You must be signed in to change notification settings - Fork 282
Convert Tests to Specs. #292
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
Changes from all commits
618570b
ed0a2a8
b7f671f
709c2b3
9e69d7c
04fd3e7
f578c4c
9c2c3e2
f655dec
1ea8499
49ed41f
9b1bd6c
8346974
bc98f3c
86d4f7b
173e177
5246535
79fcc2d
d4a1ae8
e09ebb5
d07941e
8f47b33
9bc13e2
b88903d
2b2be90
8265839
bcdd927
aa510a6
7f7cb21
2986a6b
cff180a
cbe71f8
c43cad5
3dd9c70
378b31d
87efa74
87fff3b
c6720b3
9936d06
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 |
---|---|---|
|
@@ -31,15 +31,37 @@ | |
#import "GTObject.h" | ||
|
||
|
||
@interface GTBlob : GTObject {} | ||
@interface GTBlob : GTObject | ||
|
||
+ (id)blobWithString:(NSString *)string inRepository:(GTRepository *)repository error:(NSError **)error; | ||
+ (id)blobWithData:(NSData *)data inRepository:(GTRepository *)repository error:(NSError **)error; | ||
+ (id)blobWithFile:(NSURL *)file inRepository:(GTRepository *)repository error:(NSError **)error; | ||
// Convenience class methods | ||
+ (instancetype)blobWithString:(NSString *)string inRepository:(GTRepository *)repository error:(NSError **)error; | ||
+ (instancetype)blobWithData:(NSData *)data inRepository:(GTRepository *)repository error:(NSError **)error; | ||
+ (instancetype)blobWithFile:(NSURL *)file inRepository:(GTRepository *)repository error:(NSError **)error; | ||
|
||
- (id)initWithString:(NSString *)string inRepository:(GTRepository *)repository error:(NSError **)error; | ||
- (id)initWithData:(NSData *)data inRepository:(GTRepository *)repository error:(NSError **)error; | ||
- (id)initWithFile:(NSURL *)file inRepository:(GTRepository *)repository error:(NSError **)error; | ||
// Convenience wrapper around `-initWithData:inRepository:error` that converts the string to UTF8 data | ||
- (instancetype)initWithString:(NSString *)string inRepository:(GTRepository *)repository error:(NSError **)error; | ||
|
||
// Creates a new blob from the passed data. | ||
// | ||
// This writes data to the repository's object database. | ||
// | ||
// data - The data to write. | ||
// repository - The repository to put the object in. | ||
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. May not be |
||
// error - Will be set if an error occurs. | ||
// | ||
// Returns a newly created blob object, or nil if an error occurs. | ||
- (instancetype)initWithData:(NSData *)data inRepository:(GTRepository *)repository error:(NSError **)error; | ||
|
||
// Creates a new blob from the specified file. | ||
// | ||
// This copies the data from the file to the repository's object database. | ||
// | ||
// data - The file to copy contents from. | ||
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. May not be 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. Hmm, how about we agree on defaulting to "unspecified" => "mandatory argument" ? I'm not fond of having to go through every docblock out there and add that info, and the "optional" argument case is much less frequent. 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. There are actually relatively few methods which assert paramaters as not being If something will throw an exception if you pass in 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 can see both sides of this argument, but ObjectiveGit is probably the wrong example to look at here, since it's kinda crufty in many places. I'm 👍 on @tiennou's suggestion (for all our OSS), though it'll take some time before ObjectiveGit's documentation actually gets to a state where it matches our expectations. 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'm fine with 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. It seems incredibly backwards to me that we wouldn't document something which throws an exception. |
||
// repository - The repository to put the object in. | ||
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. May not be |
||
// error - Will be set if an error occurs. | ||
// | ||
// Returns a newly created blob object, or nil if an error occurs. | ||
- (instancetype)initWithFile:(NSURL *)file inRepository:(GTRepository *)repository error:(NSError **)error; | ||
|
||
// The underlying `git_object` as a `git_blob` object. | ||
- (git_blob *)git_blob __attribute__((objc_returns_inner_pointer)); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,9 @@ + (id)blobWithFile:(NSURL *)file inRepository:(GTRepository *)repository error:( | |
} | ||
|
||
- (id)initWithOid:(const git_oid *)oid inRepository:(GTRepository *)repository error:(NSError **)error { | ||
NSParameterAssert(oid != NULL); | ||
NSParameterAssert(repository != nil); | ||
|
||
git_object *obj; | ||
int gitError = git_object_lookup(&obj, repository.git_repository, oid, (git_otype) GTObjectTypeBlob); | ||
if (gitError < GIT_OK) { | ||
|
@@ -73,6 +76,9 @@ - (id)initWithString:(NSString *)string inRepository:(GTRepository *)repository | |
} | ||
|
||
- (id)initWithData:(NSData *)data inRepository:(GTRepository *)repository error:(NSError **)error { | ||
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. Can we also document the header for this method? We should specify that |
||
NSParameterAssert(data != nil); | ||
NSParameterAssert(repository != nil); | ||
|
||
git_oid oid; | ||
int gitError = git_blob_create_frombuffer(&oid, repository.git_repository, [data bytes], data.length); | ||
if(gitError < GIT_OK) { | ||
|
@@ -86,8 +92,11 @@ - (id)initWithData:(NSData *)data inRepository:(GTRepository *)repository error: | |
} | ||
|
||
- (id)initWithFile:(NSURL *)file inRepository:(GTRepository *)repository error:(NSError **)error { | ||
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. As above. |
||
NSParameterAssert(file != nil); | ||
NSParameterAssert(repository != nil); | ||
|
||
git_oid oid; | ||
int gitError = git_blob_create_fromworkdir(&oid, repository.git_repository, [[file path] UTF8String]); | ||
int gitError = git_blob_create_fromdisk(&oid, repository.git_repository, [[file path] fileSystemRepresentation]); | ||
if(gitError < GIT_OK) { | ||
if(error != NULL) { | ||
*error = [NSError git_errorFor:gitError description:@"Failed to create blob from NSURL"]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
// | ||
// GTBlobSpec.m | ||
// ObjectiveGitFramework | ||
// | ||
// Created by Etienne Samson on 2013-11-07. | ||
// Copyright (c) 2013 GitHub, Inc. All rights reserved. | ||
// | ||
|
||
#import "GTBlob.h" | ||
|
||
SpecBegin(GTBlob) | ||
|
||
__block GTRepository *repository; | ||
__block NSString *blobSHA; | ||
__block GTBlob *blob; | ||
|
||
describe(@"blob properties can be accessed", ^{ | ||
beforeEach(^{ | ||
repository = self.bareFixtureRepository; | ||
blobSHA = @"fa49b077972391ad58037050f2a75f74e3671e92"; | ||
blob = [repository lookupObjectBySHA:blobSHA objectType:GTObjectTypeBlob error:NULL]; | ||
expect(blob).notTo.beNil(); | ||
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. We tend to prefer 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. A quick 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. Hard to see how you have 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. LOL yeah, it looks like we're actually 50/50 on this even in the GitHub for Mac codebase. |
||
}); | ||
|
||
it(@"has a size", ^{ | ||
expect(blob.size).to.equal(9); | ||
}); | ||
|
||
it(@"has content", ^{ | ||
expect(blob.content).to.equal(@"new file\n"); | ||
}); | ||
|
||
it(@"has type", ^{ | ||
expect(blob.type).to.equal(@"blob"); | ||
}); | ||
|
||
it(@"has a SHA", ^{ | ||
expect(blob.SHA).to.equal(blobSHA); | ||
}); | ||
}); | ||
|
||
describe(@"blobs can be created", ^{ | ||
beforeEach(^{ | ||
repository = self.testAppFixtureRepository; | ||
}); | ||
|
||
describe(@"+blobWithString:inRepository:error", ^{ | ||
it(@"works with valid parameters", ^{ | ||
NSError *error = nil; | ||
blob = [GTBlob blobWithString:@"a new blob content" inRepository:repository error:&error]; | ||
expect(error).to.beNil(); | ||
expect(blob).notTo.beNil(); | ||
expect(blob.SHA).notTo.beNil(); | ||
}); | ||
}); | ||
|
||
describe(@"+blobWithData:inRepository:error", ^{ | ||
it(@"works with valid parameters", ^{ | ||
char bytes[] = "100644 example_helper.rb\00\xD3\xD5\xED\x9D A4_\x00 40000 examples"; | ||
NSData *content = [NSData dataWithBytes:bytes length:sizeof(bytes)]; | ||
|
||
NSError *error = nil; | ||
blob = [GTBlob blobWithData:content inRepository:repository error:&error]; | ||
expect(error).to.beNil(); | ||
expect(blob).notTo.beNil(); | ||
expect(blob.SHA).notTo.beNil(); | ||
}); | ||
}); | ||
|
||
describe(@"+blobWithFile:inRepository:error", ^{ | ||
it(@"works with valid parameters", ^{ | ||
NSString *fileContent = @"Test contents\n"; | ||
NSString *fileName = @"myfile.txt"; | ||
NSURL *fileURL = [repository.fileURL URLByAppendingPathComponent:fileName]; | ||
|
||
NSError *error = nil; | ||
BOOL success = [fileContent writeToURL:fileURL atomically:YES encoding:NSUTF8StringEncoding error:&error]; | ||
expect(success).to.beTruthy(); | ||
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 can be written in one line. Avoiding the need for the 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'd prefer to keep this temporary for 3 reasons : it's cleaner to read, it makes the actual result appear when running under the debugger, and I'm pretty sure the style guide agrees with me ;-). But I can change it if it's really bothersome. 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. 👍 to temporaries, it's the style we've been preferring. 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. @dannygreg is it? I've only seen us do this when we're passing the @jspahrsummers @joshaber have an opinion on this? |
||
expect(error).to.beNil(); | ||
|
||
blob = [GTBlob blobWithFile:fileURL inRepository:repository error:&error]; | ||
expect(error).to.beNil(); | ||
expect(blob).notTo.beNil(); | ||
expect(blob.SHA).notTo.beNil(); | ||
expect(blob.content).to.equal(fileContent); | ||
}); | ||
}); | ||
}); | ||
|
||
SpecEnd |
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.
May not be nil.