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

Misc. small libgit2 tests #22483

Merged
merged 3 commits into from
Jun 23, 2017
Merged

Misc. small libgit2 tests #22483

merged 3 commits into from
Jun 23, 2017

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Jun 22, 2017

Add tests for:

  • Printing ConfigEntry
  • Various credential equality things (super weaksauce tests rn just to make sure these functions work)
  • head when you pass it a string that's a package location

@kshyatt kshyatt added libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests labels Jun 22, 2017
@kshyatt kshyatt requested review from omus and ararslan June 22, 2017 21:46
@@ -246,6 +246,8 @@ mktempdir() do dir
else
error("Found unexpected entry: $name")
end
show_str = sprint(show, entry)
@test show_str == string("ConfigEntry(\"", name, "\", \"", value, "\")")
Copy link
Member

Choose a reason for hiding this comment

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

Just a style issue but I find using string interpolation makes things easier to read:

"ConfigEntry(\"$name\", \"$value\")"

creds2 = LibGit2.UserPasswordCredentials(creds_user, creds_pass)
@test creds == creds2
LibGit2.reset!(creds)
@test !LibGit2.checkused!(creds)
Copy link
Member

Choose a reason for hiding this comment

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

I think having the reset! and checkused! moved up to above. Do you want this here to ensure that equality works when the count fields don't match?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that was my thought

Copy link
Member

Choose a reason for hiding this comment

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

👍

sshcreds = LibGit2.SSHCredentials(creds_user, creds_pass)
@test sshcreds.user == creds_user
@test sshcreds.pass == creds_pass
@test isempty(sshcreds.prvkey)
@test isempty(sshcreds.pubkey)
sshcreds2 = LibGit2.SSHCredentials(creds_user, creds_pass)
@test sshcreds == sshcreds2
Copy link
Member

Choose a reason for hiding this comment

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

Up to you but I would probably do:

@test sshcreds == LibGit2.SSHCredentials(creds_user, creds_pass)

@kshyatt
Copy link
Contributor Author

kshyatt commented Jun 23, 2017

@omus do you want me to change the equality test for SSHCreds or is this good to go :)?

Copy link
Member

@omus omus left a comment

Choose a reason for hiding this comment

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

We're good to go

@omus omus merged commit 7b7939c into master Jun 23, 2017
@omus omus deleted the ksh/gittests branch June 23, 2017 06:14
@ararslan
Copy link
Member

Sorry, forgot to submit a review. Retroactively LGTM.

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.

None yet

3 participants