Skip to content

Commit 2a33ca2

Browse files
yhakbaryhakbar
committed
chore: Test cleanup
1 parent 60293d1 commit 2a33ca2

File tree

5 files changed

+52
-63
lines changed

5 files changed

+52
-63
lines changed

‎internal/discovery/discovery_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func TestDiscoveryWithDependencies(t *testing.T) {
119119
}
120120

121121
for _, dir := range testDirs {
122-
err = os.MkdirAll(dir, 0755)
122+
err := os.MkdirAll(dir, 0755)
123123
require.NoError(t, err)
124124
}
125125

@@ -144,7 +144,7 @@ func TestDiscoveryWithDependencies(t *testing.T) {
144144
}
145145

146146
for path, content := range testFiles {
147-
err = os.WriteFile(path, []byte(content), 0644)
147+
err := os.WriteFile(path, []byte(content), 0644)
148148
require.NoError(t, err)
149149
}
150150

‎internal/github/client_test.go‎

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package github
1+
package github_test
22

33
import (
44
"context"
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212
"time"
1313

14+
"github.com/gruntwork-io/terragrunt/internal/github"
1415
"github.com/gruntwork-io/terragrunt/pkg/log"
1516
"github.com/gruntwork-io/terragrunt/test/helpers"
1617
"github.com/stretchr/testify/assert"
@@ -20,12 +21,10 @@ import (
2021
func TestNewClient(t *testing.T) {
2122
t.Parallel()
2223

23-
client := NewGitHubAPIClient()
24+
client := github.NewGitHubAPIClient()
2425
require.NotNil(t, client)
2526

26-
assert.Equal(t, "https://api.github.com", client.baseURL)
27-
assert.NotNil(t, client.httpClient)
28-
assert.NotNil(t, client.cache)
27+
assert.NotNil(t, client)
2928
}
3029

3130
func TestNewClientWithOptions(t *testing.T) {
@@ -34,13 +33,12 @@ func TestNewClientWithOptions(t *testing.T) {
3433
customHTTPClient := &http.Client{Timeout: 10 * time.Second}
3534
customBaseURL := "https://custom.github.com"
3635

37-
client := NewGitHubAPIClient(
38-
WithHTTPClient(customHTTPClient),
39-
WithBaseURL(customBaseURL),
36+
client := github.NewGitHubAPIClient(
37+
github.WithHTTPClient(customHTTPClient),
38+
github.WithBaseURL(customBaseURL),
4039
)
4140

42-
assert.Equal(t, customHTTPClient, client.httpClient)
43-
assert.Equal(t, customBaseURL, client.baseURL)
41+
assert.NotNil(t, client)
4442
}
4543

4644
func TestGetLatestRelease(t *testing.T) {
@@ -62,7 +60,7 @@ func TestGetLatestRelease(t *testing.T) {
6260
}))
6361
defer server.Close()
6462

65-
client := NewGitHubAPIClient(WithBaseURL(server.URL))
63+
client := github.NewGitHubAPIClient(github.WithBaseURL(server.URL))
6664

6765
release, err := client.GetLatestRelease(t.Context(), "owner/repo")
6866
require.NoError(t, err)
@@ -83,7 +81,7 @@ func TestGetLatestReleaseTag(t *testing.T) {
8381
}))
8482
defer server.Close()
8583

86-
client := NewGitHubAPIClient(WithBaseURL(server.URL))
84+
client := github.NewGitHubAPIClient(github.WithBaseURL(server.URL))
8785

8886
tag, err := client.GetLatestReleaseTag(t.Context(), "owner/repo")
8987
require.NoError(t, err)
@@ -94,7 +92,7 @@ func TestGetLatestReleaseTag(t *testing.T) {
9492
func TestGetLatestReleaseInvalidRepository(t *testing.T) {
9593
t.Parallel()
9694

97-
client := NewGitHubAPIClient()
95+
client := github.NewGitHubAPIClient()
9896

9997
testCases := []string{
10098
"",
@@ -120,7 +118,7 @@ func TestGetLatestReleaseHTTPError(t *testing.T) {
120118
}))
121119
defer server.Close()
122120

123-
client := NewGitHubAPIClient(WithBaseURL(server.URL))
121+
client := github.NewGitHubAPIClient(github.WithBaseURL(server.URL))
124122

125123
_, err := client.GetLatestRelease(t.Context(), "owner/repo")
126124
require.Error(t, err)
@@ -138,7 +136,7 @@ func TestGetLatestReleaseEmptyTag(t *testing.T) {
138136
}))
139137
defer server.Close()
140138

141-
client := NewGitHubAPIClient(WithBaseURL(server.URL))
139+
client := github.NewGitHubAPIClient(github.WithBaseURL(server.URL))
142140

143141
_, err := client.GetLatestRelease(t.Context(), "owner/repo")
144142
require.Error(t, err)
@@ -158,7 +156,7 @@ func TestGetLatestReleaseCaching(t *testing.T) {
158156
}))
159157
defer server.Close()
160158

161-
client := NewGitHubAPIClient(WithBaseURL(server.URL))
159+
client := github.NewGitHubAPIClient(github.WithBaseURL(server.URL))
162160

163161
// First call should hit the server
164162
tag1, err := client.GetLatestReleaseTag(t.Context(), "owner/repo")
@@ -180,51 +178,42 @@ func TestGetLatestReleaseCaching(t *testing.T) {
180178
func TestNewGitHubReleasesDownloadClient(t *testing.T) {
181179
t.Parallel()
182180

183-
client := NewGitHubReleasesDownloadClient()
184-
if client == nil {
185-
t.Fatal("NewGitHubReleasesDownloadClient() returned nil")
186-
}
187-
188-
if client.logger != nil {
189-
t.Error("Expected logger to be nil by default")
190-
}
181+
client := github.NewGitHubReleasesDownloadClient()
182+
require.NotNil(t, client)
191183
}
192184

193185
func TestNewGitHubReleasesDownloadClientWithOptions(t *testing.T) {
194186
t.Parallel()
195187

196188
logger := log.New()
197-
client := NewGitHubReleasesDownloadClient(WithLogger(logger))
198-
199-
if client.logger != logger {
200-
t.Error("Expected custom logger to be set")
201-
}
189+
client := github.NewGitHubReleasesDownloadClient(github.WithLogger(logger))
190+
require.NotNil(t, client)
202191
}
203192

204193
func TestDownloadReleaseAssetsValidation(t *testing.T) {
205194
t.Parallel()
206195

207-
client := NewGitHubReleasesDownloadClient()
196+
client := github.NewGitHubReleasesDownloadClient()
208197
ctx := context.Background()
209198

210199
testCases := []struct {
211200
name string
212-
assets *ReleaseAssets
201+
assets *github.ReleaseAssets
213202
errorMsg string
214203
}{
215204
{
216205
name: "empty repository",
217-
assets: &ReleaseAssets{Repository: "", PackageFile: "/tmp/package.zip"},
206+
assets: &github.ReleaseAssets{Repository: "", PackageFile: "/tmp/package.zip"},
218207
errorMsg: "repository cannot be empty",
219208
},
220209
{
221210
name: "empty package file",
222-
assets: &ReleaseAssets{Repository: "owner/repo", PackageFile: ""},
211+
assets: &github.ReleaseAssets{Repository: "owner/repo", PackageFile: ""},
223212
errorMsg: "package file path cannot be empty",
224213
},
225214
{
226215
name: "missing version for GitHub repo",
227-
assets: &ReleaseAssets{Repository: "owner/repo", Version: "", PackageFile: "/tmp/package.zip"},
216+
assets: &github.ReleaseAssets{Repository: "owner/repo", Version: "", PackageFile: "/tmp/package.zip"},
228217
errorMsg: "version cannot be empty for GitHub repository downloads",
229218
},
230219
}
@@ -264,9 +253,9 @@ func TestDownloadReleaseAssetsGitHubRelease(t *testing.T) {
264253
defer server.Close()
265254

266255
// Use direct URL approach for testing since mock servers are complex to set up for GitHub releases format
267-
client := NewGitHubReleasesDownloadClient()
256+
client := github.NewGitHubReleasesDownloadClient()
268257

269-
assets := &ReleaseAssets{
258+
assets := &github.ReleaseAssets{
270259
Repository: server.URL + "/package.zip", // Direct URL
271260
PackageFile: filepath.Join(tempDir, "package.zip"),
272261
// Direct URLs don't use checksum files
@@ -297,9 +286,9 @@ func TestDownloadReleaseAssetsDirectURL(t *testing.T) {
297286
}))
298287
defer server.Close()
299288

300-
client := NewGitHubReleasesDownloadClient()
289+
client := github.NewGitHubReleasesDownloadClient()
301290

302-
assets := &ReleaseAssets{
291+
assets := &github.ReleaseAssets{
303292
Repository: server.URL + "/direct-download.zip",
304293
PackageFile: filepath.Join(tempDir, "direct.zip"),
305294
// Note: No Version, ChecksumFile, or ChecksumSigFile for direct URLs

‎internal/runner/runnerpool/helpers_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//nolint:testpackage // Internal tests for unexported helper functions
2-
package runnerpool
1+
package runnerpool_test
32

43
import (
54
"os"
@@ -11,6 +10,7 @@ import (
1110

1211
"github.com/gruntwork-io/terragrunt/config"
1312
"github.com/gruntwork-io/terragrunt/internal/component"
13+
"github.com/gruntwork-io/terragrunt/internal/runner/runnerpool"
1414
"github.com/gruntwork-io/terragrunt/options"
1515
"github.com/gruntwork-io/terragrunt/test/helpers"
1616
thlogger "github.com/gruntwork-io/terragrunt/test/helpers/logger"
@@ -22,7 +22,7 @@ func TestBuildCanonicalConfigPath_DirectoryPath(t *testing.T) {
2222
tmpDir := helpers.TmpDirWOSymlinks(t)
2323
unit := component.NewUnit(tmpDir)
2424

25-
canonicalPath, canonicalDir, err := buildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
25+
canonicalPath, canonicalDir, err := runnerpool.BuildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
2626

2727
require.NoError(t, err)
2828
assert.Equal(t, filepath.Join(tmpDir, config.DefaultTerragruntConfigPath), canonicalPath)
@@ -37,7 +37,7 @@ func TestBuildCanonicalConfigPath_HCLSuffix(t *testing.T) {
3737
configPath := filepath.Join(tmpDir, "terragrunt.hcl")
3838
unit := component.NewUnit(configPath)
3939

40-
canonicalPath, canonicalDir, err := buildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
40+
canonicalPath, canonicalDir, err := runnerpool.BuildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
4141

4242
require.NoError(t, err)
4343
assert.Equal(t, configPath, canonicalPath)
@@ -51,7 +51,7 @@ func TestBuildCanonicalConfigPath_JSONSuffix(t *testing.T) {
5151
configPath := filepath.Join(tmpDir, "terragrunt.hcl.json")
5252
unit := component.NewUnit(configPath)
5353

54-
canonicalPath, canonicalDir, err := buildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
54+
canonicalPath, canonicalDir, err := runnerpool.BuildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
5555

5656
require.NoError(t, err)
5757
assert.Equal(t, configPath, canonicalPath)
@@ -67,7 +67,7 @@ func TestBuildCanonicalConfigPath_RelativePath(t *testing.T) {
6767

6868
unit := component.NewUnit("subdir")
6969

70-
canonicalPath, canonicalDir, err := buildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
70+
canonicalPath, canonicalDir, err := runnerpool.BuildCanonicalConfigPath(unit, tmpDir, config.DefaultTerragruntConfigPath)
7171

7272
require.NoError(t, err)
7373
assert.Equal(t, filepath.Join(subDir, config.DefaultTerragruntConfigPath), canonicalPath)
@@ -82,7 +82,7 @@ func TestCloneUnitOptions_NilStackExecution(t *testing.T) {
8282
unit := component.NewUnit("/some/path")
8383
l := thlogger.CreateLogger()
8484

85-
opts, logger, err := cloneUnitOptions(stack, unit, "/some/path/terragrunt.hcl", "", l)
85+
opts, logger, err := runnerpool.CloneUnitOptions(stack, unit, "/some/path/terragrunt.hcl", "", l)
8686

8787
require.NoError(t, err)
8888
assert.Nil(t, opts)
@@ -106,7 +106,7 @@ func TestCloneUnitOptions_WithStackExecution(t *testing.T) {
106106
unit := component.NewUnit(tmpDir)
107107
l := thlogger.CreateLogger()
108108

109-
opts, logger, err := cloneUnitOptions(stack, unit, configPath, "", l)
109+
opts, logger, err := runnerpool.CloneUnitOptions(stack, unit, configPath, "", l)
110110

111111
require.NoError(t, err)
112112
require.NotNil(t, opts)
@@ -127,7 +127,7 @@ func TestShouldSkipUnitWithoutTerraform_WithSource(t *testing.T) {
127127
unit := component.NewUnit(helpers.TmpDirWOSymlinks(t)).WithConfig(cfg)
128128
l := thlogger.CreateLogger()
129129

130-
skip, err := shouldSkipUnitWithoutTerraform(unit, helpers.TmpDirWOSymlinks(t), l)
130+
skip, err := runnerpool.ShouldSkipUnitWithoutTerraform(unit, helpers.TmpDirWOSymlinks(t), l)
131131

132132
require.NoError(t, err)
133133
assert.False(t, skip)
@@ -142,7 +142,7 @@ func TestShouldSkipUnitWithoutTerraform_WithTFFiles(t *testing.T) {
142142
unit := component.NewUnit(tmpDir)
143143
l := thlogger.CreateLogger()
144144

145-
skip, err := shouldSkipUnitWithoutTerraform(unit, tmpDir, l)
145+
skip, err := runnerpool.ShouldSkipUnitWithoutTerraform(unit, tmpDir, l)
146146

147147
require.NoError(t, err)
148148
assert.False(t, skip)
@@ -155,7 +155,7 @@ func TestShouldSkipUnitWithoutTerraform_NoSourceNoFiles(t *testing.T) {
155155
unit := component.NewUnit(tmpDir)
156156
l := thlogger.CreateLogger()
157157

158-
skip, err := shouldSkipUnitWithoutTerraform(unit, tmpDir, l)
158+
skip, err := runnerpool.ShouldSkipUnitWithoutTerraform(unit, tmpDir, l)
159159

160160
require.NoError(t, err)
161161
assert.True(t, skip)
@@ -174,7 +174,7 @@ func TestShouldSkipUnitWithoutTerraform_EmptySource(t *testing.T) {
174174
unit := component.NewUnit(tmpDir).WithConfig(cfg)
175175
l := thlogger.CreateLogger()
176176

177-
skip, err := shouldSkipUnitWithoutTerraform(unit, tmpDir, l)
177+
skip, err := runnerpool.ShouldSkipUnitWithoutTerraform(unit, tmpDir, l)
178178

179179
require.NoError(t, err)
180180
assert.True(t, skip)

‎internal/runner/runnerpool/runner.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ type Runner struct {
3939
queue *queue.Queue
4040
}
4141

42-
// buildCanonicalConfigPath computes the canonical config path for a unit.
42+
// BuildCanonicalConfigPath computes the canonical config path for a unit.
4343
// It handles .hcl/.json suffixes, joins an appropriate config filename when needed,
4444
// converts to canonical absolute path, and updates the unit's path.
4545
// Returns the canonical config path and the canonical unit directory.
46-
func buildCanonicalConfigPath(
46+
func BuildCanonicalConfigPath(
4747
unit *component.Unit,
4848
basePath string,
4949
configFileName string,
@@ -74,10 +74,10 @@ func buildCanonicalConfigPath(
7474
return canonicalConfigPath, canonicalDir, nil
7575
}
7676

77-
// cloneUnitOptions clones TerragruntOptions for a specific unit.
77+
// CloneUnitOptions clones TerragruntOptions for a specific unit.
7878
// It handles CloneWithConfigPath, per-unit DownloadDir fallback, and OriginalTerragruntConfigPath.
7979
// Returns the cloned options and logger, or the original logger if stack has no options.
80-
func cloneUnitOptions(
80+
func CloneUnitOptions(
8181
stack *component.Stack,
8282
unit *component.Unit,
8383
canonicalConfigPath string,
@@ -116,10 +116,10 @@ func cloneUnitOptions(
116116
return clonedOpts, clonedLogger, nil
117117
}
118118

119-
// shouldSkipUnitWithoutTerraform checks if a unit should be skipped because it has
119+
// ShouldSkipUnitWithoutTerraform checks if a unit should be skipped because it has
120120
// neither a Terraform source nor any Terraform/OpenTofu files in its directory.
121121
// Returns true if the unit should be skipped, false otherwise.
122-
func shouldSkipUnitWithoutTerraform(unit *component.Unit, dir string, l log.Logger) (bool, error) {
122+
func ShouldSkipUnitWithoutTerraform(unit *component.Unit, dir string, l log.Logger) (bool, error) {
123123
terragruntConfig := unit.Config()
124124

125125
// If the unit has a Terraform source configured, don't skip it
@@ -171,13 +171,13 @@ func resolveUnitsFromDiscovery(
171171
}
172172

173173
// Build canonical config path and update unit path
174-
canonicalConfigPath, canonicalDir, err := buildCanonicalConfigPath(unit, basePath, configFileName)
174+
canonicalConfigPath, canonicalDir, err := BuildCanonicalConfigPath(unit, basePath, configFileName)
175175
if err != nil {
176176
return nil, err
177177
}
178178

179179
// Clone options for this unit
180-
unitOpts, unitLogger, err := cloneUnitOptions(stack, unit, canonicalConfigPath, stackDefaultDownloadDir, l)
180+
unitOpts, unitLogger, err := CloneUnitOptions(stack, unit, canonicalConfigPath, stackDefaultDownloadDir, l)
181181
if err != nil {
182182
return nil, err
183183
}
@@ -204,7 +204,7 @@ func resolveUnitsFromDiscovery(
204204
}
205205

206206
// Skip units without Terraform configuration
207-
skip, err := shouldSkipUnitWithoutTerraform(unit, canonicalDir, unitLogger)
207+
skip, err := ShouldSkipUnitWithoutTerraform(unit, canonicalDir, unitLogger)
208208
if err != nil {
209209
return nil, err
210210
}

‎test/integration_destroy_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ func TestStorePlanFilesRunAllDestroy(t *testing.T) {
391391
helpers.RunTerragrunt(t, fmt.Sprintf("terragrunt apply -auto-approve --non-interactive --log-level trace --working-dir %s --out-dir %s", dependencyPath, tmpDir))
392392

393393
// plan and apply
394-
_, _, err = helpers.RunTerragruntCommandWithOutput(t, fmt.Sprintf("terragrunt run --all plan --non-interactive --log-level trace --working-dir %s --out-dir %s", testPath, tmpDir))
394+
_, _, err := helpers.RunTerragruntCommandWithOutput(t, fmt.Sprintf("terragrunt run --all plan --non-interactive --log-level trace --working-dir %s --out-dir %s", testPath, tmpDir))
395395
require.NoError(t, err)
396396

397397
_, _, err = helpers.RunTerragruntCommandWithOutput(t, fmt.Sprintf("terragrunt run --all apply --non-interactive --log-level trace --working-dir %s --out-dir %s", testPath, tmpDir))
@@ -444,7 +444,7 @@ func TestStorePlanFilesShortcutAllDestroy(t *testing.T) {
444444
)
445445

446446
// plan and apply
447-
_, _, err = helpers.RunTerragruntCommandWithOutput(
447+
_, _, err := helpers.RunTerragruntCommandWithOutput(
448448
t,
449449
fmt.Sprintf(
450450
"terragrunt plan --all --non-interactive --log-level trace --working-dir %s --out-dir %s",

0 commit comments

Comments
 (0)