[chore] Cleanup chloggen globalCfg usage (#376)

open-telemetry · Jul 25, 2023 · 37d80b2 · 37d80b2
1 parent 4216b12
commit 37d80b2
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 31 deletions.
diff --git a/chloggen/cmd/cmd_test.go b/chloggen/cmd/cmd_test.go
@@ -98,36 +98,28 @@ func entryWithSubtext() *chlog.Entry {
 	}
 }
 
-func setupTestDir(t *testing.T, entries []*chlog.Entry) config.Config {
-	cfg := config.New(t.TempDir())
+func setupTestDir(t *testing.T, entries []*chlog.Entry) {
+	require.NotNil(t, globalCfg, "test should instantiate globalCfg before calling setupTestDir")
 
 	// Create a known dummy changelog which may be updated by the test
 	changelogBytes, err := os.ReadFile(filepath.Join("testdata", config.DefaultChangelogMD))
 	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(cfg.ChangelogMD, changelogBytes, os.FileMode(0755)))
+	require.NoError(t, os.WriteFile(globalCfg.ChangelogMD, changelogBytes, os.FileMode(0755)))
 
-	require.NoError(t, os.Mkdir(cfg.ChlogsDir, os.FileMode(0755)))
+	require.NoError(t, os.Mkdir(globalCfg.ChlogsDir, os.FileMode(0755)))
 
 	// Copy the entry template, for tests that ensure it is not deleted
 	templateInRootDir := config.New("testdata").TemplateYAML
 	templateBytes, err := os.ReadFile(filepath.Clean(templateInRootDir))
 	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(cfg.TemplateYAML, templateBytes, os.FileMode(0755)))
+	require.NoError(t, os.WriteFile(globalCfg.TemplateYAML, templateBytes, os.FileMode(0755)))
 
 	for i, entry := range entries {
-		require.NoError(t, writeEntryYAML(cfg, fmt.Sprintf("%d.yaml", i), entry))
+		entryBytes, err := yaml.Marshal(entry)
+		require.NoError(t, err)
+		path := filepath.Join(globalCfg.ChlogsDir, fmt.Sprintf("%d.yaml", i))
+		require.NoError(t, os.WriteFile(path, entryBytes, os.FileMode(0755)))
 	}
-
-	return cfg
-}
-
-func writeEntryYAML(cfg config.Config, filename string, entry *chlog.Entry) error {
-	entryBytes, err := yaml.Marshal(entry)
-	if err != nil {
-		return err
-	}
-	path := filepath.Join(cfg.ChlogsDir, filename)
-	return os.WriteFile(path, entryBytes, os.FileMode(0755))
 }
 
 func runCobra(t *testing.T, args ...string) (string, string) {

diff --git a/chloggen/cmd/new_test.go b/chloggen/cmd/new_test.go
@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"go.opentelemetry.io/build-tools/chloggen/internal/chlog"
+	"go.opentelemetry.io/build-tools/chloggen/internal/config"
 )
 
 const newUsage = `Usage:
@@ -51,7 +52,8 @@ func TestNewErr(t *testing.T) {
 }
 
 func TestNew(t *testing.T) {
-	globalCfg = setupTestDir(t, []*chlog.Entry{})
+	globalCfg = config.New(t.TempDir())
+	setupTestDir(t, []*chlog.Entry{})
 
 	var out, err string
 

diff --git a/chloggen/cmd/root.go b/chloggen/cmd/root.go
@@ -25,7 +25,7 @@ import (
 
 var (
 	configFile string
-	globalCfg  config.Config
+	globalCfg  *config.Config
 )
 
 func rootCmd() *cobra.Command {
@@ -51,8 +51,7 @@ func init() {
 
 func initConfig() {
 	// Don't override if already set in tests
-	var uninitialized config.Config
-	if globalCfg != uninitialized {
+	if globalCfg != nil {
 		return
 	}
 

diff --git a/chloggen/cmd/update_test.go b/chloggen/cmd/update_test.go
@@ -25,6 +25,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/build-tools/chloggen/internal/chlog"
+	"go.opentelemetry.io/build-tools/chloggen/internal/config"
 )
 
 const updateUsage = `Usage:
@@ -39,6 +40,9 @@ Global Flags:
       --config string   (optional) chloggen config file`
 
 func TestUpdateErr(t *testing.T) {
+	globalCfg = config.New(t.TempDir())
+	setupTestDir(t, []*chlog.Entry{})
+
 	var out, err string
 
 	out, err = runCobra(t, "update", "--help")
@@ -49,7 +53,6 @@ func TestUpdateErr(t *testing.T) {
 	assert.Contains(t, out, updateUsage)
 	assert.Contains(t, err, "no entries to add to the changelog")
 
-	globalCfg = setupTestDir(t, []*chlog.Entry{})
 	badEntry, ioErr := os.CreateTemp(globalCfg.ChlogsDir, "*.yaml")
 	require.NoError(t, ioErr)
 	defer badEntry.Close()
@@ -122,7 +125,8 @@ func TestUpdate(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			globalCfg = setupTestDir(t, tc.entries)
+			globalCfg = config.New(t.TempDir())
+			setupTestDir(t, tc.entries)
 
 			args := []string{"update", "--version", tc.version}
 			if tc.dry {

diff --git a/chloggen/cmd/validate_test.go b/chloggen/cmd/validate_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"go.opentelemetry.io/build-tools/chloggen/internal/chlog"
+	"go.opentelemetry.io/build-tools/chloggen/internal/config"
 )
 
 const validateUsage = `Usage:
@@ -140,7 +141,8 @@ func TestValidate(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			globalCfg = setupTestDir(t, tc.entries)
+			globalCfg = config.New(t.TempDir())
+			setupTestDir(t, tc.entries)
 
 			out, err := runCobra(t, "validate")
 

diff --git a/chloggen/internal/chlog/entry.go b/chloggen/internal/chlog/entry.go
@@ -93,7 +93,7 @@ func (e Entry) String() string {
 	return sb.String()
 }
 
-func ReadEntries(cfg config.Config) ([]*Entry, error) {
+func ReadEntries(cfg *config.Config) ([]*Entry, error) {
 	entryYAMLs, err := filepath.Glob(filepath.Join(cfg.ChlogsDir, "*.yaml"))
 	if err != nil {
 		return nil, err
@@ -119,7 +119,7 @@ func ReadEntries(cfg config.Config) ([]*Entry, error) {
 	return entries, nil
 }
 
-func DeleteEntries(cfg config.Config) error {
+func DeleteEntries(cfg *config.Config) error {
 	entryYAMLs, err := filepath.Glob(filepath.Join(cfg.ChlogsDir, "*.yaml"))
 	if err != nil {
 		return err

diff --git a/chloggen/internal/config/config.go b/chloggen/internal/config/config.go
@@ -35,23 +35,23 @@ type Config struct {
 	ConfigYAML   string
 }
 
-func New(rootDir string) Config {
-	return Config{
+func New(rootDir string) *Config {
+	return &Config{
 		ChangelogMD:  filepath.Join(rootDir, DefaultChangelogMD),
 		ChlogsDir:    filepath.Join(rootDir, DefaultChloggenDir),
 		TemplateYAML: filepath.Join(rootDir, DefaultChloggenDir, DefaultTemplateYAML),
 	}
 }
 
-func NewFromFile(rootDir string, filename string) (Config, error) {
+func NewFromFile(rootDir string, filename string) (*Config, error) {
 	cfg := New(rootDir)
 	cfg.ConfigYAML = filepath.Clean(filepath.Join(rootDir, filename))
 	cfgBytes, err := os.ReadFile(cfg.ConfigYAML)
 	if err != nil {
-		return Config{}, err
+		return nil, err
 	}
 	if err = yaml.Unmarshal(cfgBytes, &cfg); err != nil {
-		return Config{}, err
+		return nil, err
 	}
 
 	// If the user specified any of the following, interpret as a relative path from rootDir