Skip to content

Added tests for Branch#1181

Merged
johnhaley81 merged 2 commits intonodegit:masterfrom
rcjsuen:branch-tests
Jan 28, 2017
Merged

Added tests for Branch#1181
johnhaley81 merged 2 commits intonodegit:masterfrom
rcjsuen:branch-tests

Conversation

@rcjsuen
Copy link
Copy Markdown
Member

@rcjsuen rcjsuen commented Jan 6, 2017

move
lookup
createFromAnnotated

Note that the test for createFromAnnotated is failing for me. It seems like the branch is being created even when the force flag has not been set. I would have expected the conflict in the branch's name to prevent the tag from being created but that's not happening.

The returned annotated commit does have the correct SHA-1 hash so that is not the issue. The branch that gets returned by the promise from createFromAnnotated has the same SHA-1 hash as what it was originally pointing at (ergo, the master commit) so it's almost as if the function call never happened! 🤔

The function was originally synchronous which meant it would return
something on regardless of whether it succeeded (a branch) or failed
(an error code). This forces the client to have to check whether the
returned object is an error code or an object. Change the function to
be asynchronous instead so that clients can use the resolved or
rejected promise to easily determine whether the branch was
successfully created or not.
move
lookup
createFromAnnotated
@rcjsuen
Copy link
Copy Markdown
Member Author

rcjsuen commented Jan 28, 2017

I realized that the error was due to the function being synchronous and always returning something regardless of whether the function succeeded or failed. This caused the then(*) function to be called even though it had actually failed (which then caused the test to fail). Changing the function to be asynchronous now gets the test to pass successfully.

@johnhaley81
Copy link
Copy Markdown
Collaborator

Thanks @rcjsuen!

@johnhaley81 johnhaley81 merged commit 0cff089 into nodegit:master Jan 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants