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.

Sign up for GitHub

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

Jump to bottom

Generate LibGit2 credential test expressions #23668

Merged
merged 1 commit into from
Sep 15, 2017
Merged

Generate LibGit2 credential test expressions #23668

merged 1 commit into from
Sep 15, 2017

Conversation

omus
Copy link
Member

@omus omus commented Sep 11, 2017

Generate credential test expressions instead of writing them all by hand. Makes it easier to deal with refactoring.

Only possible since #23575 and #23641 were merged. Spin off of the work done in #20725.

@omus omus added libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests labels Sep 11, 2017
@omus omus force-pushed the cv/libgit2-gen-test branch from 14821f4 to 38b1136 Compare September 13, 2017 04:40
@omus
Copy link
Member Author

omus commented Sep 13, 2017 •
edited
Loading

The Circle CI failure at first appears related but I think it may just be a coincidence as these tests work fine everywhere else and also worked before they were rebased.

Worker 4 terminated.
ERROR (unhandled task failure): EOFError: read end of file
...
libgit2: Error During Test
  Test threw an exception of type ProcessExitedException
  Expression: libgit2
  ProcessExitedException()

rofinn
rofinn approved these changes Sep 14, 2017
View reviewed changes
Copy link
Contributor

@rofinn rofinn left a comment

Choose a reason for hiding this comment

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

I think this looks good, but I was a little unclear on a comment deletion. FWIW, it was definitely easier to follow the new code :)

test/libgit2.jl
@test err == git_ok
@test auth_attempts == 1

# Explicitly provided credential is incorrect
# TODO: Unless the SSH agent is disabled we may get caught in an infinite loop
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this no longer an issue?

Copy link
Member Author

@omus omus Sep 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Was fixed in #23641 when use_ssh_agent=false was set. I discovered the outdated comment while updating the tests:

julia/test/libgit2.jl

Line 1971 in c172e92

credential_loop(valid_cred, $url, $username, payload, use_ssh_agent=false)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, okay, thanks.

@omus omus merged commit a16e374 into master Sep 15, 2017
@omus omus deleted the cv/libgit2-gen-test branch September 15, 2017 18:46
omus added a commit that referenced this pull request Sep 18, 2017
@omus
Credential prompt abort shouldn't modify the cache
2ea3e35 
Requires:
- Approval/rejection (#23711)
- Allow prompt keyword (#23690)

Helpful:
- Generate tests (#23668)

I would apply this commit to the approval/rejection PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rofinn rofinn rofinn approved these changes

Assignees
No one assigned
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@omus @rofinn