Add tests for the existing config #8213

williammartin · 2023-10-18T15:39:54Z

Description

In preparation for changing our configuration schema to support multiple accounts, I want gain confidence in our Config, particularly the pieces that relate to the AuthConfig structure and reach down into the config and keyring. I actually couldn't find any tests that provided coverage for this even as an integration.

There will need to also be some tests added to the go-gh/auth package which is called through here and reaches into the config.

I've left some comments on areas I want to call out.

williammartin · 2023-10-18T15:53:57Z

internal/config/auth_config_test.go

+
+	// When the user has logged in insecurely
+	authCfg := newTestAuthConfig()
+	ghConfig.Read = func() (*ghConfig.Config, error) {


A bit sad 😬

I dont think this is necessary:

cfg, err := NewConfig() require.NoError(t, err) authCfg := cfg.Authentication()

I don't think what you're proposing here actually resolves my sadness so just to be clear about why I'm doing this NewConfig calls to ghConfig.Read which uses sync.Once to load config from disk. That means that any test from that point onward that calls ghConfig.Read will not be isolated.

Ah I see. I misunderstood the purpose. That makes sense to me.

I think what I'll do is actually put it inside the newTestAuthConfig function or another new function and comment it so that the reason can be discovered from any test.

williammartin · 2023-10-18T15:54:38Z

internal/config/auth_config_test.go

+	require.ErrorIs(t, err, keyring.ErrNotFound)
+}
+
+// Note that I'm not sure this test enforces particularly desirable behaviour


Calling this out for attention.

The reasoning behind this behavior was that if logout fails what actions can the user actually take? The thought was that there really was none and thus we shouldn't show them an error. Not saying I agree, just some context.

Depends on the user I suppose. A user that cares that their token is still hanging around on a shared machine could care.

I don't feel a pressing need to change it but I wanted to leave a note for any maintainer that comes across this in future. In fact, I'll update the comment to capture your added context.

williammartin · 2023-10-18T15:55:02Z

internal/config/config.go

 	if err == nil {
 		return val, err
 	}

-	if defaultExists(key) {


I dunno, feel free to tell me to move it back but it felt pretty analogous to a map.

I like the change.

williammartin · 2023-10-18T15:55:11Z

internal/config/config.go

@@ -178,13 +160,19 @@ func (c *AuthConfig) User(hostname string) (string, error) {

 // GitProtocol will retrieve the git protocol for the logged in user at the given hostname.
 // If none is set it will return the default value.
+// TODO: although this returns an error, it actually has no path to error.


Calling this out for attention.

Let's change the function signature then, it is only used in two places. Interestingly enough this function should replace the majority of places GetOrDefault is used. Would love to clean that up as I find GetOrDefault to be a bit of an eyesore.

williammartin · 2023-10-18T15:55:16Z

internal/config/config.go

@@ -245,6 +235,8 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 // Logout will remove user, git protocol, and auth token for the given hostname.
 // It will remove the auth token from the encrypted storage if it exists there.
 func (c *AuthConfig) Logout(hostname string) error {
+	// TODO: I can't find anywhere that expects to call this function without a hostname


Calling this out for attention.

I think this was in place for legacy reasons, probably okay to remove now.

williammartin · 2023-10-18T15:56:09Z

internal/config/config.go

@@ -233,6 +221,8 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 		c.cfg.Set([]string{hosts, hostname, oauthToken}, token)
 		insecureStorageUsed = true
 	}
+	// TODO: I can't find anywhere that calls this function without a username, so that would seem like


Calling this out for attention.

I think this was in place for legacy reasons, probably okay to remove now.

williammartin · 2023-10-18T16:12:10Z

internal/config/auth_config_test.go

@@ -0,0 +1,302 @@
+package config


Note that I have not written tests for KnownHosts (because it has no extra behaviour on top of go-gh/auth right now so it was low priority) nor the AliasConfig because it is low risk for the multi account changes.

I have written tests for Hosts and DefaultHost now but I will leave AliasConfig out of this PR and do it in a follow up since this is already getting pretty hefty.

williammartin · 2023-10-18T16:13:24Z

internal/config/auth_config_test.go

+}
+
+func TestHasEnvTokenWithNoEnvTokenButAConfigVar(t *testing.T) {
+	t.Skip("this test is explicitly breaking some implementation assumptions")


Calling this out for attention.

williammartin · 2023-10-18T16:13:32Z

internal/config/config.go

@@ -153,6 +135,14 @@ func (c *AuthConfig) HasEnvToken() bool {
 			return true
 		}
 	}
+	// TODO: This is _extremely_ knowledgable about the implementation of TokenFromEnvOrConfig


Calling this out for attention.

Agreed. We should create a function HasEnvToken in go-gh that abstracts away this knowledge.

I'm going to punt that from this PR since it's a bigger change and this is already pretty hefty.

williammartin · 2023-10-19T07:41:46Z

internal/config/auth_config_test.go

+	requireKeyWithValue(t, authCfg.cfg, []string{hosts, "github.com", "user"}, "test-user")
+	requireKeyWithValue(t, authCfg.cfg, []string{hosts, "github.com", "git_protocol"}, "ssh")


I should test these using GitProtocol and User instead. That would be a better black box test.

samcoe

@williammartin This is excellent, thanks for being diligent about improving our test coverage! I left inline code comments, let me know if you have any questions about them.

samcoe · 2023-10-19T08:04:40Z

internal/config/auth_config_test.go

+
+	// When the user has logged in insecurely
+	authCfg := newTestAuthConfig()
+	ghConfig.Read = func() (*ghConfig.Config, error) {


I dont think this is necessary:

cfg, err := NewConfig() require.NoError(t, err) authCfg := cfg.Authentication()

samcoe · 2023-10-19T08:12:56Z

internal/config/auth_config_test.go

+
+	// When the user is authenticated via env var
+	authCfg := newTestAuthConfig()
+	ghConfig.Read = func() (*ghConfig.Config, error) {


Same comment as above.

samcoe · 2023-10-19T08:21:21Z

internal/config/auth_config_test.go

+	require.ErrorIs(t, err, keyring.ErrNotFound)
+}
+
+// Note that I'm not sure this test enforces particularly desirable behaviour


The reasoning behind this behavior was that if logout fails what actions can the user actually take? The thought was that there really was none and thus we shouldn't show them an error. Not saying I agree, just some context.

samcoe · 2023-10-19T08:22:08Z

internal/config/config.go

 	if err == nil {
 		return val, err
 	}

-	if defaultExists(key) {


I like the change.

samcoe · 2023-10-19T08:23:14Z

internal/config/config.go

-	}
-
-	val, err = c.cfg.Get([]string{key})
+	val, err := c.Get(hostname, key)


samcoe · 2023-10-19T08:25:41Z

internal/config/config.go

@@ -153,6 +135,14 @@ func (c *AuthConfig) HasEnvToken() bool {
 			return true
 		}
 	}
+	// TODO: This is _extremely_ knowledgable about the implementation of TokenFromEnvOrConfig


Agreed. We should create a function HasEnvToken in go-gh that abstracts away this knowledge.

samcoe · 2023-10-19T08:27:00Z

internal/config/config.go

+	// can guarantee that tokens will only be returned from a set env var.
+	// Discussed here, but maybe worth revisiting: https://github.com/cli/cli/pull/7169#discussion_r1136979033
+	//
+	// By providing example.com. it's also _only_ looking for GH_ENTERPRISE_TOKEN or GITHUB_ENTERPRISE_TOKEN


It checks GH_TOKEN and GITHUB_TOKEN as well.

Ohhh sorry, I missed that the isEnterprise branch of tokenForHost would fall through. Thanks for correcting me!

samcoe · 2023-10-19T08:31:56Z

internal/config/config.go

@@ -178,13 +160,19 @@ func (c *AuthConfig) User(hostname string) (string, error) {

 // GitProtocol will retrieve the git protocol for the logged in user at the given hostname.
 // If none is set it will return the default value.
+// TODO: although this returns an error, it actually has no path to error.


Let's change the function signature then, it is only used in two places. Interestingly enough this function should replace the majority of places GetOrDefault is used. Would love to clean that up as I find GetOrDefault to be a bit of an eyesore.

samcoe · 2023-10-19T08:33:27Z

internal/config/config.go

@@ -233,6 +221,8 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 		c.cfg.Set([]string{hosts, hostname, oauthToken}, token)
 		insecureStorageUsed = true
 	}
+	// TODO: I can't find anywhere that calls this function without a username, so that would seem like


I think this was in place for legacy reasons, probably okay to remove now.

samcoe · 2023-10-19T08:33:41Z

internal/config/config.go

@@ -245,6 +235,8 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 // Logout will remove user, git protocol, and auth token for the given hostname.
 // It will remove the auth token from the encrypted storage if it exists there.
 func (c *AuthConfig) Logout(hostname string) error {
+	// TODO: I can't find anywhere that expects to call this function without a hostname


I think this was in place for legacy reasons, probably okay to remove now.

This is to avoid the impression that the keyring matters, when the test is actually about general auth behaviour

williammartin · 2023-10-19T11:00:19Z

I believe I've addressed all comments @samcoe and have explicitly excluded:

Testing AliasConfig
Moving HasEnvToken into gh-auth

In both cases, I felt like this PR was already pretty hefty and they were both larger distractions from the multi account confidence that I wanted to get out of this.

samcoe

Wonderful! Thanks for making the requested changes. This looks ready to ship. Glad we are one step closer to removing GetOrDefault.

samcoe · 2023-10-19T12:54:25Z

internal/config/auth_config_test.go

+
+	// Then the token is successfully fetched
+	// and the source is set to oauth_token but this isn't great
+	// but I can't find the issue # that references this.


I think cli/go-gh#94 is what you are looking for.

Yes! I just could not find it anywhere

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [aquaproj/aqua-registry](https://togithub.com/aquaproj/aqua-registry) | minor | `v4.75.0` -> `v4.79.0` | | [bitnami-labs/sealed-secrets](https://togithub.com/bitnami-labs/sealed-secrets) | patch | `v0.24.2` -> `v0.24.3` | | [cli/cli](https://togithub.com/cli/cli) | minor | `v2.37.0` -> `v2.38.0` | | [golangci/golangci-lint](https://togithub.com/golangci/golangci-lint) | patch | `v1.55.1` -> `v1.55.2` | | [kevincobain2000/gobrew](https://togithub.com/kevincobain2000/gobrew) | patch | `1.9.3` -> `v1.9.6` | | [nektos/act](https://togithub.com/nektos/act) | patch | `v0.2.52` -> `v0.2.53` | | [twpayne/chezmoi](https://togithub.com/twpayne/chezmoi) | patch | `v2.40.3` -> `v2.40.4` | --- ### Release Notes <details> <summary>aquaproj/aqua-registry (aquaproj/aqua-registry)</summary> ### [`v4.79.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.79.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.78.0...v4.79.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.79.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.79.0) | aquaproj/aqua-registry@v4.78.0...v4.79.0 #### 🎉 New Packages [#16956](https://togithub.com/aquaproj/aqua-registry/issues/16956) [#16958](https://togithub.com/aquaproj/aqua-registry/issues/16958) [crates.io/tailspin](https://crates.io/crates/tailspin): A log file highlighter [#16920](https://togithub.com/aquaproj/aqua-registry/issues/16920) [int128/cronjob-runner](https://togithub.com/int128/cronjob-runner): A command to run one-shot job from CronJob template and tail container logs in Kubernetes #### Fixes [#16961](https://togithub.com/aquaproj/aqua-registry/issues/16961) Boeing/config-file-validator: Follow up changes of validator v1.5.0 [#16964](https://togithub.com/aquaproj/aqua-registry/issues/16964) kptdev/kpt: Rename GoogleContainerTools/kpt and exclude versions with the prefix `porch/` https://github.com/GoogleContainerTools/kpt is redirected to https://github.com/kptdev/kpt [#16965](https://togithub.com/aquaproj/aqua-registry/issues/16965) bitnami-labs/sealed-secrets: Exclude versions with the prefix `helm-` ### [`v4.78.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.78.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.77.0...v4.78.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.78.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.78.0) | aquaproj/aqua-registry@v4.77.0...v4.78.0 #### 🎉 New Packages [#16835](https://togithub.com/aquaproj/aqua-registry/issues/16835) [manabusakai/tdtidy](https://togithub.com/manabusakai/tdtidy): A command line tool for managing ECS task definitions. `tdtidy` can deregister and delete old task definitions [@ponkio-o](https://togithub.com/ponkio-o) #### Fixes [#16916](https://togithub.com/aquaproj/aqua-registry/issues/16916) deepmap/oapi-codegen: Support oapi-codegen v2 [#16913](https://togithub.com/aquaproj/aqua-registry/issues/16913) hktalent/scan4all: Follow up changes of scan4all 2.8.6 Asset names were changed. GhostTroops/scan4all@40d6c24 ### [`v4.77.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.77.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.76.0...v4.77.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.77.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.77.0) | aquaproj/aqua-registry@v4.76.0...v4.77.0 #### 🎉 Reached 1,300 packages 🎉 Thank you, all contributors! #### 🎉 New Packages [#16765](https://togithub.com/aquaproj/aqua-registry/issues/16765) [traefik/yaegi](https://togithub.com/traefik/yaegi): Yaegi is Another Elegant Go Interpreter [#16755](https://togithub.com/aquaproj/aqua-registry/issues/16755) [#16756](https://togithub.com/aquaproj/aqua-registry/issues/16756) [xeol-io/xeol](https://togithub.com/xeol-io/xeol): A scanner for end-of-life (EOL) software in container images, filesystems, and SBOMs ### [`v4.76.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.76.0) [Compare Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.75.0...v4.76.0) [Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.76.0) | [Pull Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.76.0) | aquaproj/aqua-registry@v4.75.0...v4.76.0 #### 🎉 New Packages [#16680](https://togithub.com/aquaproj/aqua-registry/issues/16680) [astral-sh/ruff](https://togithub.com/astral-sh/ruff): An extremely fast Python linter and code formatter, written in Rust #### Fixes [#16749](https://togithub.com/aquaproj/aqua-registry/issues/16749) mozilla/sccache: Follow up changes of sccache v0.6.0 arch64-unknown-linux-musl got disabled. - [mozilla/sccache#1917 - https://github.com/mozilla/sccache/releases/tag/v0.6.0 [#16750](https://togithub.com/aquaproj/aqua-registry/issues/16750) kastenhq/external-tools/k10tools: Fix settings [#16751](https://togithub.com/aquaproj/aqua-registry/issues/16751) kastenhq/external-tools/k10multicluster: Fix settings [#16752](https://togithub.com/aquaproj/aqua-registry/issues/16752) google/osv-scanner: Follow up an issue of osv-scanner v1.4.2 - [google/osv-scanner#611 </details> <details> <summary>bitnami-labs/sealed-secrets (bitnami-labs/sealed-secrets)</summary> ### [`v0.24.3`](https://togithub.com/bitnami-labs/sealed-secrets/blob/HEAD/RELEASE-NOTES.md#v0243) [Compare Source](https://togithub.com/bitnami-labs/sealed-secrets/compare/v0.24.2...v0.24.3) ##### Changelog - fix a bug that kept a sealed secret's generation and observedgeneration out of sync ([#1360](https://togithub.com/bitnami-labs/sealed-secrets/pull/1360)) - fix: add pdb ([#1340](https://togithub.com/bitnami-labs/sealed-secrets/pull/1340)) - Bump k8s.io/code-generator from 0.28.2 to 0.28.3 ([#1358](https://togithub.com/bitnami-labs/sealed-secrets/pull/1340)) - Bump github.com/onsi/gomega from 1.28.1 to 1.29.0 ([#1357](https://togithub.com/bitnami-labs/sealed-secrets/pull/1357)) - Bump github.com/mattn/go-isatty from 0.0.19 to 0.0.20 ([#1353](https://togithub.com/bitnami-labs/sealed-secrets/pull/1353)) - Bump github.com/onsi/gomega from 1.28.0 to 1.28.1 ([#1351](https://togithub.com/bitnami-labs/sealed-secrets/pull/1351)) - Bump k8s.io/client-go from 0.28.2 to 0.28.3 ([#1350](https://togithub.com/bitnami-labs/sealed-secrets/pull/1350)) - Bump k8s.io/api from 0.28.2 to 0.28.3 ([#1349](https://togithub.com/bitnami-labs/sealed-secrets/pull/1349)) - Bump github.com/google/go-cmp from 0.5.9 to 0.6.0 ([#1348](https://togithub.com/bitnami-labs/sealed-secrets/pull/1348)) </details> <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.38.0`](https://togithub.com/cli/cli/releases/tag/v2.38.0): GitHub CLI 2.38.0 [Compare Source](https://togithub.com/cli/cli/compare/v2.37.0...v2.38.0) #### Highlights - `extension install` no longer errors unhelpfully if the extension is already installed by [@Delta456](https://togithub.com/Delta456) in [cli/cli#8211 - All tables now have headers by [@heaths](https://togithub.com/heaths) in [cli/cli#8157 - `project` commands have a clearer error message when no owner can be resolved by [@ffalor](https://togithub.com/ffalor) in [cli/cli#8235 - `workflow run` now presents a select for `choice` workflow input types by [@adarshjhaa100](https://togithub.com/adarshjhaa100) in [cli/cli#8180 - `codespace create` no longer polls for additional codespace permissions unnecessarily by [@dmgardiner25](https://togithub.com/dmgardiner25) in [cli/cli#8267 - `go install` now works with the removal of our crypto fork by [@samcoe](https://togithub.com/samcoe) in [cli/cli#8204 #### Everything Else - Additional testing for config by [@williammartin](https://togithub.com/williammartin) in [cli/cli#8213 - Bumped cpuguy83/go-md2man from 2.0.1 to 2.0.3 by [@mikelolasagasti](https://togithub.com/mikelolasagasti) in [cli/cli#8209 - Bumped mattn/go-isatty from 0.0.19 to 0.0.20 by [@dependabot](https://togithub.com/dependabot) in [cli/cli#8205 - Bumped google.golang.org/grpc from 1.53.0 to 1.56.3 by [@dependabot](https://togithub.com/dependabot) in [cli/cli#8251 - Bumped creack/pty from 1.1.18 to 1.1.20 by [@dependabot](https://togithub.com/dependabot) in [cli/cli#8265 - Provide default config to bumped `go-gh` by [@samcoe](https://togithub.com/samcoe) in [cli/cli#8244 #### New Contributors - [@mikelolasagasti](https://togithub.com/mikelolasagasti) made their first contribution in [cli/cli#8209 - [@Delta456](https://togithub.com/Delta456) made their first contribution in [cli/cli#8211 - [@adarshjhaa100](https://togithub.com/adarshjhaa100) made their first contribution in [cli/cli#8180 **Full Changelog**: cli/cli@v2.37.0...v2.38.0 </details> <details> <summary>golangci/golangci-lint (golangci/golangci-lint)</summary> ### [`v1.55.2`](https://togithub.com/golangci/golangci-lint/compare/v1.55.1...v1.55.2) [Compare Source](https://togithub.com/golangci/golangci-lint/compare/v1.55.1...v1.55.2) </details> <details> <summary>kevincobain2000/gobrew (kevincobain2000/gobrew)</summary> ### [`v1.9.6`](https://togithub.com/kevincobain2000/gobrew/releases/tag/v1.9.6) [Compare Source](https://togithub.com/kevincobain2000/gobrew/compare/v1.9.5...v1.9.6) #### Changelog - [`99135bd`](https://togithub.com/kevincobain2000/gobrew/commit/99135bd) Merge pull request [#150](https://togithub.com/kevincobain2000/gobrew/issues/150) from kevincobain2000/feature/gobrew-mod-major - [`1dbbb5f`](https://togithub.com/kevincobain2000/gobrew/commit/1dbbb5f) README updated - [`5f566b0`](https://togithub.com/kevincobain2000/gobrew/commit/5f566b0) only support go.mod major versions ### [`v1.9.5`](https://togithub.com/kevincobain2000/gobrew/releases/tag/v1.9.5): - Interactive [Compare Source](https://togithub.com/kevincobain2000/gobrew/compare/v1.9.4...v1.9.5) #### Changelog - [`b612228`](https://togithub.com/kevincobain2000/gobrew/commit/b612228) Merge pull request [#148](https://togithub.com/kevincobain2000/gobrew/issues/148) from kevincobain2000/feature/gobrew-bug - [`c6d7844`](https://togithub.com/kevincobain2000/gobrew/commit/c6d7844) oops - [`3f80b5a`](https://togithub.com/kevincobain2000/gobrew/commit/3f80b5a) oops bug ### [`v1.9.4`](https://togithub.com/kevincobain2000/gobrew/releases/tag/v1.9.4): - Interactive [Compare Source](https://togithub.com/kevincobain2000/gobrew/compare/1.9.3...v1.9.4) #### Changelog ![Screenshot 2023-11-01 at 17 59 11](https://togithub.com/kevincobain2000/gobrew/assets/629055/5ec75b5b-6fab-45da-82d4-5ad0203b2ac5) - [`ddb42bf`](https://togithub.com/kevincobain2000/gobrew/commit/ddb42bf) (CHANGE LOG) updated readme - [`94cd26a`](https://togithub.com/kevincobain2000/gobrew/commit/94cd26a) (ci) fix test - [`4003742`](https://togithub.com/kevincobain2000/gobrew/commit/4003742) (ut) skip one failing test for now - [`82dc507`](https://togithub.com/kevincobain2000/gobrew/commit/82dc507) (vup) go.mod to 1.21 - [`7d911bf`](https://togithub.com/kevincobain2000/gobrew/commit/7d911bf) Fixes [#145](https://togithub.com/kevincobain2000/gobrew/issues/145) by adding interactivity - [`4aabed8`](https://togithub.com/kevincobain2000/gobrew/commit/4aabed8) Merge pull request [#143](https://togithub.com/kevincobain2000/gobrew/issues/143) from lincolnthalles/powershell-install-script - [`defdbff`](https://togithub.com/kevincobain2000/gobrew/commit/defdbff) Merge pull request [#144](https://togithub.com/kevincobain2000/gobrew/issues/144) from kevincobain2000/feature/ci-and-others - [`1859f1e`](https://togithub.com/kevincobain2000/gobrew/commit/1859f1e) Merge pull request [#147](https://togithub.com/kevincobain2000/gobrew/issues/147) from kevincobain2000/feature/gobrew - [`d900ad5`](https://togithub.com/kevincobain2000/gobrew/commit/d900ad5) README updated - [`4c850a8`](https://togithub.com/kevincobain2000/gobrew/commit/4c850a8) Revert "(ci) fix test" - [`c18716e`](https://togithub.com/kevincobain2000/gobrew/commit/c18716e) Support when no current version - [`b48f97f`](https://togithub.com/kevincobain2000/gobrew/commit/b48f97f) Update README.md - [`dcd8063`](https://togithub.com/kevincobain2000/gobrew/commit/dcd8063) chore: add powershell install script - [`310b5a3`](https://togithub.com/kevincobain2000/gobrew/commit/310b5a3) chore: fix powershell script - [`01e22fa`](https://togithub.com/kevincobain2000/gobrew/commit/01e22fa) chore: fix powershell script - [`c6b0877`](https://togithub.com/kevincobain2000/gobrew/commit/c6b0877) chore: update powershell script - [`2ae12de`](https://togithub.com/kevincobain2000/gobrew/commit/2ae12de) chore: update powershell script - [`38a053e`](https://togithub.com/kevincobain2000/gobrew/commit/38a053e) getting version from go.mod may throw error when there is no go.mod file. check for file existance first, otherwise set to None - [`d39e076`](https://togithub.com/kevincobain2000/gobrew/commit/d39e076) redundant comment gone - [`ed6fb34`](https://togithub.com/kevincobain2000/gobrew/commit/ed6fb34) screenshot - [`f691f10`](https://togithub.com/kevincobain2000/gobrew/commit/f691f10) tests++ for interactive </details> <details> <summary>nektos/act (nektos/act)</summary> ### [`v0.2.53`](https://togithub.com/nektos/act/releases/tag/v0.2.53) [Compare Source](https://togithub.com/nektos/act/compare/v0.2.52...v0.2.53) #### Changelog ##### Bug fixes - [`7c7d80e`](https://togithub.com/nektos/act/commit/7c7d80e) fix: use actions/runner hashfiles in container ([#1940](https://togithub.com/nektos/act/issues/1940)) ##### Other - [`1bb2ee7`](https://togithub.com/nektos/act/commit/1bb2ee7) chore: bump VERSION to 0.2.53 - [`84a4025`](https://togithub.com/nektos/act/commit/84a4025) build(deps): bump github.com/docker/docker ([#2067](https://togithub.com/nektos/act/issues/2067)) - [`fb4f29f`](https://togithub.com/nektos/act/commit/fb4f29f) build(deps): bump github.com/creack/pty from 1.1.18 to 1.1.20 ([#2068](https://togithub.com/nektos/act/issues/2068)) - [`3e5c629`](https://togithub.com/nektos/act/commit/3e5c629) build(deps): bump megalinter/megalinter from 7.4.0 to 7.5.0 ([#2070](https://togithub.com/nektos/act/issues/2070)) - [`83bfbcd`](https://togithub.com/nektos/act/commit/83bfbcd) build(deps): bump go.etcd.io/bbolt from 1.3.7 to 1.3.8 ([#2065](https://togithub.com/nektos/act/issues/2065)) - [`3d65b0f`](https://togithub.com/nektos/act/commit/3d65b0f) build(deps): bump github.com/docker/cli ([#2069](https://togithub.com/nektos/act/issues/2069)) - [`854e3e9`](https://togithub.com/nektos/act/commit/854e3e9) build(deps): bump github.com/go-git/go-git/v5 from 5.9.0 to 5.10.0 ([#2066](https://togithub.com/nektos/act/issues/2066)) - [`db71c41`](https://togithub.com/nektos/act/commit/db71c41) build(deps): bump github.com/mattn/go-isatty from 0.0.19 to 0.0.20 ([#2059](https://togithub.com/nektos/act/issues/2059)) - [`db6e477`](https://togithub.com/nektos/act/commit/db6e477) build(deps): bump github.com/moby/buildkit from 0.12.2 to 0.12.3 ([#2060](https://togithub.com/nektos/act/issues/2060)) - [`ceeb6c1`](https://togithub.com/nektos/act/commit/ceeb6c1) Add support for service containers ([#1949](https://togithub.com/nektos/act/issues/1949)) - [`ace4cd4`](https://togithub.com/nektos/act/commit/ace4cd4) Fix float formatting ([#2018](https://togithub.com/nektos/act/issues/2018)) - [`99067a9`](https://togithub.com/nektos/act/commit/99067a9) build(deps): bump golang.org/x/net from 0.15.0 to 0.17.0 ([#2045](https://togithub.com/nektos/act/issues/2045)) - [`e7e158c`](https://togithub.com/nektos/act/commit/e7e158c) build(deps): bump github.com/docker/distribution ([#2037](https://togithub.com/nektos/act/issues/2037)) - [`3c730d7`](https://togithub.com/nektos/act/commit/3c730d7) build(deps): bump golang.org/x/term from 0.12.0 to 0.13.0 ([#2036](https://togithub.com/nektos/act/issues/2036)) - [`976df8b`](https://togithub.com/nektos/act/commit/976df8b) fix action_ref (composite action) ([#2020](https://togithub.com/nektos/act/issues/2020)) - [`2f479ba`](https://togithub.com/nektos/act/commit/2f479ba) Fix image survey for large images ([#2022](https://togithub.com/nektos/act/issues/2022)) - [`5718555`](https://togithub.com/nektos/act/commit/5718555) \[ Variables ] - Add missing documentation for repository variables ([#2032](https://togithub.com/nektos/act/issues/2032)) </details> <details> <summary>twpayne/chezmoi (twpayne/chezmoi)</summary> ### [`v2.40.4`](https://togithub.com/twpayne/chezmoi/releases/tag/v2.40.4) [Compare Source](https://togithub.com/twpayne/chezmoi/compare/v2.40.3...v2.40.4) #### Changelog ##### Fixes - [`797e3cf`](https://togithub.com/twpayne/chezmoi/commit/797e3cf0f) fix: Make stdinIsATTY return false if --no-tty is passed </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 4pm on thursday" in timezone America/Los_Angeles, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] 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/scottames/dots).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

williammartin added 7 commits October 18, 2023 15:51

Add initial tests for config Get

b94f458

Add tests for config GetOrDefault

5f645ea

Add initial AuthConfig tests

34b0b33

Add tests for AuthConfig Login and Logout

614e492

Update AuthConfig to use default fn

52ba99e

Add tests for AuthConfig Token

07110ec

Mod tidy go-md2man

465474e

williammartin force-pushed the wm/add-tests-for-existing-config branch from b80aca5 to 465474e Compare October 18, 2023 15:52

Add tests for AuthConfig HasEnvToken

e011633

williammartin commented Oct 18, 2023

View reviewed changes

williammartin requested a review from samcoe October 18, 2023 16:14

williammartin commented Oct 19, 2023

View reviewed changes

samcoe reviewed Oct 19, 2023

View reviewed changes

williammartin added 12 commits October 19, 2023 12:05

Rewrite AuthConfig User and GitProtocol tests

3380831

Add additional comment on Logout ignoring errors

cb7672e

Add test that host is added on Login

e0bc2ff

Share config.Read setup in AuthConfig tests

07f5ca4

Add tests for AuthConfig DefaultHost

fa80802

Avoid using keyring unnecessarily in AuthConfig tests

c9de817

This is to avoid the impression that the keyring matters, when the test is actually about general auth behaviour

Ensure all AuthConfig tests are isolated on disk

c430da7

Remove Logout branch that handles empty hostname

aa88484

Remove Login branch that handles empty username

3c3fbcd

Remove incorrect comment about HasEnvToken

b9cbee0

Fix GitProtocol signature and rework test descriptions

7d8c1af

Remove GetOrDefault uses in favour of GitProtocol

6dc04bb

williammartin marked this pull request as ready for review October 19, 2023 10:58

williammartin requested a review from a team as a code owner October 19, 2023 10:58

cliAutomation assigned williammartin Oct 19, 2023

cliAutomation added this to Needs review 🤔 in The GitHub CLI Oct 19, 2023

samcoe approved these changes Oct 19, 2023

View reviewed changes

Add link to token source issue

4fc1999

williammartin merged commit 26f3601 into trunk Oct 19, 2023
10 checks passed

williammartin deleted the wm/add-tests-for-existing-config branch October 19, 2023 13:59

samcoe mentioned this pull request Oct 25, 2023

Globally set git protocol is ignored in some places #8250

Closed

		requireKeyWithValue(t, authCfg.cfg, []string{hosts, "github.com", "user"}, "test-user")
		requireKeyWithValue(t, authCfg.cfg, []string{hosts, "github.com", "git_protocol"}, "ssh")

Add tests for the existing config #8213

Add tests for the existing config #8213

Conversation

williammartin commented Oct 18, 2023 • edited

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williammartin Oct 19, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samcoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williammartin commented Oct 19, 2023

samcoe left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williammartin commented Oct 18, 2023 •

edited

williammartin Oct 19, 2023 •

edited

samcoe left a comment •

edited