-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Rework first auth tests with new gitcredential abstractions #9095
Rework first auth tests with new gitcredential abstractions #9095
Conversation
@@ -33,105 +35,40 @@ func configureTestCredentialHelper(t *testing.T, key string) { | |||
require.NoError(t, cmd.Run()) | |||
} | |||
|
|||
func TestConfigureOursNoPreexistingHelpersConfigured(t *testing.T) { |
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.
These became part of the contract
// Then gh is configured as their credential helper for that host | ||
ios, _, _, _ := iostreams.Test() | ||
|
||
reg := &httpmock.Registry{} |
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.
I'm happy to look into figuring out how this test would fit into the table tests but I wanted it to be very clear in exactly what it was doing for this review.
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.
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://togithub.com/cli/cli) | minor | `v2.49.2` -> `v2.50.0` | --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.50.0`](https://togithub.com/cli/cli/releases/tag/v2.50.0): GitHub CLI 2.50.0 [Compare Source](https://togithub.com/cli/cli/compare/v2.49.2...v2.50.0) #### What's Changed - Refactor git credential flow code by [@​williammartin](https://togithub.com/williammartin) in [cli/cli#9089 - feat: add json output for `gh pr checks` by [@​nobe4](https://togithub.com/nobe4) in [cli/cli#9079 - Rework first auth tests with new gitcredential abstractions by [@​williammartin](https://togithub.com/williammartin) in [cli/cli#9095 - list the various alias permutations for the command and subcommands, via '--help' and 'gh reference' by [@​gabemontero](https://togithub.com/gabemontero) in [cli/cli#8824 - Removed tty message when checking for extension upgrades by [@​leevic31](https://togithub.com/leevic31) in [cli/cli#9088 - Fix doc bug for gh run watch by [@​jasonodonnell](https://togithub.com/jasonodonnell) in [cli/cli#9052 - feat: add support for stateReason in `gh pr view` by [@​nobe4](https://togithub.com/nobe4) in [cli/cli#9080 - fix: rename the `Attempts` field to `Attempt`; expose in `gh run view` and `gh run ls` by [@​cawfeecake](https://togithub.com/cawfeecake) in [cli/cli#8905 - Update regex in changedFilesNames to handle quoted paths by [@​anda3](https://togithub.com/anda3) in [cli/cli#9115 - Add a `gh variable get FOO` command by [@​arnested](https://togithub.com/arnested) in [cli/cli#9106 - Add macOS pkg installer to deployment ([#​7554](https://togithub.com/cli/cli/issues/7554)) by [@​paulober](https://togithub.com/paulober) in [cli/cli#7555 - Add integration tests for `gh attestation verify` shared workflow use case by [@​malancas](https://togithub.com/malancas) in [cli/cli#9107 - Add build provenance for gh CLI releases by [@​malancas](https://togithub.com/malancas) in [cli/cli#9087 - build(deps): bump github.com/gabriel-vasile/mimetype from 1.4.3 to 1.4.4 by [@​dependabot](https://togithub.com/dependabot) in [cli/cli#9124 - Build completions during release on macos by [@​williammartin](https://togithub.com/williammartin) in [cli/cli#9136 - Clarify Mac OS Installer packages are unsigned by [@​andyfeller](https://togithub.com/andyfeller) in [cli/cli#9140 #### New Contributors - [@​gabemontero](https://togithub.com/gabemontero) made their first contribution in [cli/cli#8824 - [@​jasonodonnell](https://togithub.com/jasonodonnell) made their first contribution in [cli/cli#9052 - [@​anda3](https://togithub.com/anda3) made their first contribution in [cli/cli#9115 - [@​arnested](https://togithub.com/arnested) made their first contribution in [cli/cli#9106 - [@​paulober](https://togithub.com/paulober) made their first contribution in [cli/cli#7555 **Full Changelog**: cli/cli@v2.49.2...v2.50.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/izumin5210/dotfiles). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNzcuOCIsInVwZGF0ZWRJblZlciI6IjM3LjM3Ny44IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: izumin5210-update-aqua-checksum[bot] <169593670+izumin5210-update-aqua-checksum[bot]@users.noreply.github.com>
Description
This is a follow up to #9089 as progress towards #8875.
In #9089 new abstractions were introduced to more clearly separate the configuration of git credentials configuration, and informing the helpers of updated credentials.
This PR now cleaves
GitCredentialFlow
andgitcredentials.HelperConfig
by introducing aHelperConfig
interface on the consumer side. We introduce a contract test that can be run by concrete implementations of this interface to ensure they are able to be test doubled with confidence. That is, a newgitcredentials.FakeHelperConfig
can be used in place of thegitcredentials.HelperConfig
without requiringgit
and an isolated config (instead, that is only needed for theHelperConfig
at the leaf of the dependency graph).This
FakeHelperConfig
can then be used in theLoginFlow
tests, of which I have created exactly one to demonstrate this pattern. The purpose of this new test is to ensure that when no credential helper is found for the targeted host, the user is presented with a prompt, and when they accept that prompt,gh
is configured as their credential helper.Summed up, it makes this branch go from red to green:
Login Flow:
And therefore all of this GitCredentialFlow code goes green too:
Why now?
In #8875 I want to lean on these abstractions to ensure that
auth switch
exhibits similar behaviour but it should be easier to develop and review in smaller incremental steps demonstrating the pattern with existing code.