Skip to content
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

Add some tests for libgit2 and a function to list all the reference names #16728

Merged
merged 4 commits into from
Jun 8, 2016

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Jun 2, 2016

@kshyatt kshyatt added the test This change adds or pertains to unit tests label Jun 2, 2016
@wildart
Copy link
Member

wildart commented Jun 3, 2016

👍

@@ -219,6 +219,12 @@ mktempdir() do dir
@test LibGit2.iszero(commit_oid2)
commit_oid2 = LibGit2.commit(repo, commit_msg2; author=test_sig, committer=test_sig)
@test !LibGit2.iszero(commit_oid2)
auths = LibGit2.authors(repo)
for auth in auths
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how many should there be in this test case?

@kshyatt kshyatt force-pushed the ksh/gitreftests branch from db6bca4 to 4761fc6 Compare June 3, 2016 04:15
@kshyatt
Copy link
Contributor Author

kshyatt commented Jun 3, 2016

@tkelman this good to go?

@@ -219,6 +219,13 @@ mktempdir() do dir
@test LibGit2.iszero(commit_oid2)
commit_oid2 = LibGit2.commit(repo, commit_msg2; author=test_sig, committer=test_sig)
@test !LibGit2.iszero(commit_oid2)
auths = LibGit2.authors(repo)
@test length(auths) == 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which repo is this, is it constructed from scratch in this test harness, or is it referring to something like Example.jl that may change over time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure it's from scratch
On Jun 3, 2016 10:43 AM, "Tony Kelman" notifications@github.com wrote:

In test/libgit2.jl
#16728 (comment):

@@ -219,6 +219,13 @@ mktempdir() do dir
@test LibGit2.iszero(commit_oid2)
commit_oid2 = LibGit2.commit(repo, commit_msg2; author=test_sig, committer=test_sig)
@test !LibGit2.iszero(commit_oid2)

  •            auths = LibGit2.authors(repo)
    
  •            @test length(auths) == 3
    

which repo is this, is it constructed from scratch in this test harness,
or is it referring to something like Example.jl that may change over time?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/JuliaLang/julia/pull/16728/files/4761fc6757a8ce5b5d8a049737ca7bc2a0293976#r65747279,
or mute the thread
https://github.com/notifications/unsubscribe/AAyk452oBDr8VRy17iIXaMEOESjv-t9eks5qIGejgaJpZM4Is-63
.

@kshyatt kshyatt added the libgit2 The libgit2 library or the LibGit2 stdlib module label Jun 6, 2016
@kshyatt
Copy link
Contributor Author

kshyatt commented Jun 6, 2016

I don't think the OSX failure here has anything to do with me. @tkelman and @wildart, does this look ok?

return oid_ptr[]
oid_ptr = ccall((:git_tag_target_id, :libgit2), Ptr{Oid}, (Ptr{Void}, ), tag.ptr)
oid_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
return Oid(oid_ptr)
Copy link
Contributor

@tkelman tkelman Jun 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh in this case the Oid constructor actually does check and throw for null pointers already.

Just so I'm clear, the old code was returning a Ptr{Oid} from the ccall, wrapping that in a Ref then dereferencing the Ref - what type was that returning, a Ptr{Oid} or an Oid?

edit: nevermind I was looking at the wrong Oid constructor, the Oid(ptr::Ptr{Oid}) just does unsafe_load(ptr)::Oid so leave the null check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should return an Oid, I think. @wildart ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I never used this function, but I'm pretty sure that it should return Oid. My implementation was wrong.

@tkelman tkelman merged commit 615f7ca into master Jun 8, 2016
@tkelman tkelman deleted the ksh/gitreftests branch June 8, 2016 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants