Repository: apigee/registry
Pull request #784 (2022-10-14 18:48:35)

Allow overriding the registry tool configuration in tests.
This includes modifications to the config package that allow the registry.project to be forced to particular values during testing.

I'm not attached to the implementation, this is just one way to pursue a goal of testing the registry tool under different configurations without modifying files in possibly-surprising ways.

theganyo: I'd suggest just having a function called "SetConfig()" that sets the config pointer and call it a config, not a cache. I don't believe we need the useCache or clearConfig or anything like that. If the config pointer is populated, we use it; if not, we attempt to load the config.
timburks: > I'd suggest just having a function called "SetConfig()" that sets the config pointer and call it a config, not a cache. I don't believe we need the useCache or clearConfig or anything like that. If the config pointer is populated, we use it; if not, we attempt to load the config.

@theganyo This seems to mean that when we set any config value in code, we would need to set all of the config values. I think in tests we might want to set the project but not touch the configured server address.

theganyo: I see... So I think what I hear you saying is that you'd like to load and keep certain external values (from, I assume, env vars), but then override other specific values for specific tests. In that case, why not just get the config, modify the values you want to change, and then SetConfig() back?
timburks: That sounds good, as long as `SetConfig()` doesn't write to the configuration file -- I don't think we want this to have side effects outside the tests.
codecov[bot]: # [Codecov](https://codecov.io/gh/apigee/registry/pull/784?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) Report
> Merging [#784](https://codecov.io/gh/apigee/registry/pull/784?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (a1891af) into [main](https://codecov.io/gh/apigee/registry/commit/dbc994d535b1d6e82baeb9bc7c72482f0e538d17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) (dbc994d) will **increase** coverage by `0.00%`.
> The diff coverage is `0.00%`.

> :exclamation: Current head a1891af differs from pull request most recent head d9f5bcd. Consider uploading reports for the commit d9f5bcd to get more accurate results

```diff
@@           Coverage Diff           @@
##             main     #784   +/-   ##
=======================================
  Coverage   55.84%   55.85%           
=======================================
  Files          97       97           
  Lines        8532     8537    +5     
=======================================
+ Hits         4765     4768    +3     
  Misses       3290     3290           
- Partials      477      479    +2     
```


| [Impacted Files](https://codecov.io/gh/apigee/registry/pull/784?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee) | Coverage Δ | |
|---|---|---|
| [pkg/connection/config.go](https://codecov.io/gh/apigee/registry/pull/784/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-cGtnL2Nvbm5lY3Rpb24vY29uZmlnLmdv) | `61.90% <0.00%> (-8.37%)` | :arrow_down: |
| [cmd/registry/cmd/apply/apply.go](https://codecov.io/gh/apigee/registry/pull/784/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee#diff-Y21kL3JlZ2lzdHJ5L2NtZC9hcHBseS9hcHBseS5nbw==) | `63.15% <0.00%> (+7.89%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apigee)

timburks: I'll go with `active` on your joint recommendation. Thanks!

Patch:
diff --git a/pkg/connection/config.go b/pkg/connection/config.go
index 7da9b10..3887dd1 100644
--- a/pkg/connection/config.go
+++ b/pkg/connection/config.go
@@ -31,12 +31,26 @@ type Config struct {
 	Token    string `mapstructure:"token"`    // bearer token
 }
 
+// If set, ActiveConfig() returns this configuration.
+// This is intended for use in testing.
+var active *Config
+
+// SetConfig forces the active configuration to use the specified value.
+func SetConfig(config Config) {
+	active = &config
+}
+
 // ActiveConfig returns the active config.
 func ActiveConfig() (Config, error) {
+	if active != nil {
+		return *active, nil
+	}
+
 	name, err := config.ActiveName()
 	if err != nil {
 		return Config{}, err
 	}
+
 	return ReadConfig(name)
 }
 


Test patch:
diff --git a/cmd/registry/cmd/apply/apply_test.go b/cmd/registry/cmd/apply/apply_test.go
index f2ff53d..0247c7a 100644
--- a/cmd/registry/cmd/apply/apply_test.go
+++ b/cmd/registry/cmd/apply/apply_test.go
@@ -173,6 +173,14 @@ func TestApplyProject(t *testing.T) {
 		t.Fatalf("Setup: Failed to create test project: %s", err)
 	}
 
+	// set the configured registry.project to the test project
+	config, err := connection.ActiveConfig()
+	if err != nil {
+		t.Fatalf("Setup: Failed to get registry configuration: %s", err)
+	}
+	config.Project = project.ProjectID
+	connection.SetConfig(config)
+
 	registryClient, err := connection.NewRegistryClient(ctx)
 	if err != nil {
 		t.Fatalf("Setup: Failed to create registry client: %s", err)
@@ -180,7 +188,7 @@ func TestApplyProject(t *testing.T) {
 	defer registryClient.Close()
 
 	cmd := Command()
-	cmd.SetArgs([]string{"-f", sampleDir, "-R", "--parent", project.String() + "/locations/global"})
+	cmd.SetArgs([]string{"-f", sampleDir, "-R"})
 	if err := cmd.Execute(); err != nil {
 		t.Fatalf("Execute() with args %+v returned error: %s", cmd.Args, err)
 	}