Skip to content

Commit e0b8b3b

Browse files
denis256denis256
committed
chore: filter tests cleanup
1 parent 029a923 commit e0b8b3b

File tree

5 files changed

+72
-57
lines changed

5 files changed

+72
-57
lines changed

‎internal/discovery/worktreediscovery.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ func (wd *WorktreeDiscovery) Discover(
8484
discoveryGroup.Go(func() error {
8585
// Track filter expansion with telemetry
8686
var fromFilters, toFilters filter.Filters
87+
8788
diffs := pair.Diffs
8889

8990
expandErr := filter.TraceGitFilterExpand(
@@ -304,7 +305,9 @@ func (wd *WorktreeDiscovery) walkChangedStack(
304305
toStack.DiscoveryContext().Ref,
305306
func(ctx context.Context) error {
306307
var walkErr error
308+
307309
result, walkErr = wd.walkChangedStackInternal(ctx, l, opts, originalDiscovery, fromStack, toStack)
310+
308311
return walkErr
309312
},
310313
)

‎internal/filter/telemetry.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,11 @@ func TraceGitWorktreeCreate(ctx context.Context, ref, worktreeDir, repoRemote, r
6767
if repoRemote != "" {
6868
attrs[AttrGitRepoRemote] = repoRemote
6969
}
70+
7071
if repoBranch != "" {
7172
attrs[AttrGitRepoBranch] = repoBranch
7273
}
74+
7375
if repoCommit != "" {
7476
attrs[AttrGitRepoCommit] = repoCommit
7577
}
@@ -104,9 +106,11 @@ func TraceGitWorktreesCreate(ctx context.Context, workingDir string, refCount in
104106
if repoRemote != "" {
105107
attrs[AttrGitRepoRemote] = repoRemote
106108
}
109+
107110
if repoBranch != "" {
108111
attrs[AttrGitRepoBranch] = repoBranch
109112
}
113+
110114
if repoCommit != "" {
111115
attrs[AttrGitRepoCommit] = repoCommit
112116
}

‎internal/filter/telemetry_test.go‎

Lines changed: 57 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,165 +1,167 @@
1-
package filter
1+
package filter_test
22

33
import (
44
"context"
55
"testing"
66

7+
"github.com/gruntwork-io/terragrunt/internal/filter"
78
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
810
)
911

1012
func TestTraceGitWorktreeCreate_NoTelemeter(t *testing.T) {
1113
t.Parallel()
1214

1315
called := false
14-
err := TraceGitWorktreeCreate(context.Background(), "main", "/tmp/worktree", "[email protected]:org/repo.git", "main", "abc123", func(ctx context.Context) error {
16+
err := filter.TraceGitWorktreeCreate(context.Background(), "main", "/tmp/worktree", "[email protected]:org/repo.git", "main", "abc123", func(ctx context.Context) error {
1517
called = true
1618
return nil
1719
})
1820

19-
assert.NoError(t, err)
21+
require.NoError(t, err)
2022
assert.True(t, called, "callback should be called even without telemeter")
2123
}
2224

2325
func TestTraceGitWorktreeRemove_NoTelemeter(t *testing.T) {
2426
t.Parallel()
2527

2628
called := false
27-
err := TraceGitWorktreeRemove(context.Background(), "main", "/tmp/worktree", func(ctx context.Context) error {
29+
err := filter.TraceGitWorktreeRemove(context.Background(), "main", "/tmp/worktree", func(ctx context.Context) error {
2830
called = true
2931
return nil
3032
})
3133

32-
assert.NoError(t, err)
34+
require.NoError(t, err)
3335
assert.True(t, called, "callback should be called even without telemeter")
3436
}
3537

3638
func TestTraceGitWorktreesCreate_NoTelemeter(t *testing.T) {
3739
t.Parallel()
3840

3941
called := false
40-
err := TraceGitWorktreesCreate(context.Background(), "/work", 2, "[email protected]:org/repo.git", "main", "abc123", func(ctx context.Context) error {
42+
err := filter.TraceGitWorktreesCreate(context.Background(), "/work", 2, "[email protected]:org/repo.git", "main", "abc123", func(ctx context.Context) error {
4143
called = true
4244
return nil
4345
})
4446

45-
assert.NoError(t, err)
47+
require.NoError(t, err)
4648
assert.True(t, called, "callback should be called even without telemeter")
4749
}
4850

4951
func TestTraceGitWorktreesCleanup_NoTelemeter(t *testing.T) {
5052
t.Parallel()
5153

5254
called := false
53-
err := TraceGitWorktreesCleanup(context.Background(), 2, "[email protected]:org/repo.git", func(ctx context.Context) error {
55+
err := filter.TraceGitWorktreesCleanup(context.Background(), 2, "[email protected]:org/repo.git", func(ctx context.Context) error {
5456
called = true
5557
return nil
5658
})
5759

58-
assert.NoError(t, err)
60+
require.NoError(t, err)
5961
assert.True(t, called, "callback should be called even without telemeter")
6062
}
6163

6264
func TestTraceGitDiff_NoTelemeter(t *testing.T) {
6365
t.Parallel()
6466

6567
called := false
66-
err := TraceGitDiff(context.Background(), "main", "HEAD", "[email protected]:org/repo.git", func(ctx context.Context) error {
68+
err := filter.TraceGitDiff(context.Background(), "main", "HEAD", "[email protected]:org/repo.git", func(ctx context.Context) error {
6769
called = true
6870
return nil
6971
})
7072

71-
assert.NoError(t, err)
73+
require.NoError(t, err)
7274
assert.True(t, called, "callback should be called even without telemeter")
7375
}
7476

7577
func TestTraceGitWorktreeDiscovery_NoTelemeter(t *testing.T) {
7678
t.Parallel()
7779

7880
called := false
79-
err := TraceGitWorktreeDiscovery(context.Background(), 3, func(ctx context.Context) error {
81+
err := filter.TraceGitWorktreeDiscovery(context.Background(), 3, func(ctx context.Context) error {
8082
called = true
8183
return nil
8284
})
8385

84-
assert.NoError(t, err)
86+
require.NoError(t, err)
8587
assert.True(t, called, "callback should be called even without telemeter")
8688
}
8789

8890
func TestTraceGitWorktreeStackWalk_NoTelemeter(t *testing.T) {
8991
t.Parallel()
9092

9193
called := false
92-
err := TraceGitWorktreeStackWalk(context.Background(), "main", "feature", func(ctx context.Context) error {
94+
err := filter.TraceGitWorktreeStackWalk(context.Background(), "main", "feature", func(ctx context.Context) error {
9395
called = true
9496
return nil
9597
})
9698

97-
assert.NoError(t, err)
99+
require.NoError(t, err)
98100
assert.True(t, called, "callback should be called even without telemeter")
99101
}
100102

101103
func TestTraceFilterEvaluate_NoTelemeter(t *testing.T) {
102104
t.Parallel()
103105

104106
called := false
105-
err := TraceFilterEvaluate(context.Background(), 5, 10, func(ctx context.Context) error {
107+
err := filter.TraceFilterEvaluate(context.Background(), 5, 10, func(ctx context.Context) error {
106108
called = true
107109
return nil
108110
})
109111

110-
assert.NoError(t, err)
112+
require.NoError(t, err)
111113
assert.True(t, called, "callback should be called even without telemeter")
112114
}
113115

114116
func TestTraceFilterParse_NoTelemeter(t *testing.T) {
115117
t.Parallel()
116118

117119
called := false
118-
err := TraceFilterParse(context.Background(), "name=foo", func(ctx context.Context) error {
120+
err := filter.TraceFilterParse(context.Background(), "name=foo", func(ctx context.Context) error {
119121
called = true
120122
return nil
121123
})
122124

123-
assert.NoError(t, err)
125+
require.NoError(t, err)
124126
assert.True(t, called, "callback should be called even without telemeter")
125127
}
126128

127129
func TestTraceGitFilterExpand_NoTelemeter(t *testing.T) {
128130
t.Parallel()
129131

130132
called := false
131-
err := TraceGitFilterExpand(context.Background(), "main", "HEAD", 3, 1, 5, func(ctx context.Context) error {
133+
err := filter.TraceGitFilterExpand(context.Background(), "main", "HEAD", 3, 1, 5, func(ctx context.Context) error {
132134
called = true
133135
return nil
134136
})
135137

136-
assert.NoError(t, err)
138+
require.NoError(t, err)
137139
assert.True(t, called, "callback should be called even without telemeter")
138140
}
139141

140142
func TestTraceGitFilterEvaluate_NoTelemeter(t *testing.T) {
141143
t.Parallel()
142144

143145
called := false
144-
err := TraceGitFilterEvaluate(context.Background(), "main", "HEAD", 10, func(ctx context.Context) error {
146+
err := filter.TraceGitFilterEvaluate(context.Background(), "main", "HEAD", 10, func(ctx context.Context) error {
145147
called = true
146148
return nil
147149
})
148150

149-
assert.NoError(t, err)
151+
require.NoError(t, err)
150152
assert.True(t, called, "callback should be called even without telemeter")
151153
}
152154

153155
func TestTraceGraphFilterTraverse_NoTelemeter(t *testing.T) {
154156
t.Parallel()
155157

156158
called := false
157-
err := TraceGraphFilterTraverse(context.Background(), "dependencies", 5, func(ctx context.Context) error {
159+
err := filter.TraceGraphFilterTraverse(context.Background(), "dependencies", 5, func(ctx context.Context) error {
158160
called = true
159161
return nil
160162
})
161163

162-
assert.NoError(t, err)
164+
require.NoError(t, err)
163165
assert.True(t, called, "callback should be called even without telemeter")
164166
}
165167

@@ -168,22 +170,23 @@ func TestTelemetryConstants(t *testing.T) {
168170

169171
// Verify operation names are unique and well-formed
170172
opNames := []string{
171-
TelemetryOpGitWorktreeCreate,
172-
TelemetryOpGitWorktreeRemove,
173-
TelemetryOpGitWorktreesCreate,
174-
TelemetryOpGitWorktreesCleanup,
175-
TelemetryOpGitDiff,
176-
TelemetryOpGitWorktreeDiscovery,
177-
TelemetryOpGitWorktreeStackWalk,
178-
TelemetryOpGitWorktreeFilterApply,
179-
TelemetryOpFilterEvaluate,
180-
TelemetryOpFilterParse,
181-
TelemetryOpGitFilterExpand,
182-
TelemetryOpGitFilterEvaluate,
183-
TelemetryOpGraphFilterTraverse,
173+
filter.TelemetryOpGitWorktreeCreate,
174+
filter.TelemetryOpGitWorktreeRemove,
175+
filter.TelemetryOpGitWorktreesCreate,
176+
filter.TelemetryOpGitWorktreesCleanup,
177+
filter.TelemetryOpGitDiff,
178+
filter.TelemetryOpGitWorktreeDiscovery,
179+
filter.TelemetryOpGitWorktreeStackWalk,
180+
filter.TelemetryOpGitWorktreeFilterApply,
181+
filter.TelemetryOpFilterEvaluate,
182+
filter.TelemetryOpFilterParse,
183+
filter.TelemetryOpGitFilterExpand,
184+
filter.TelemetryOpGitFilterEvaluate,
185+
filter.TelemetryOpGraphFilterTraverse,
184186
}
185187

186188
seen := make(map[string]bool)
189+
187190
for _, name := range opNames {
188191
assert.NotEmpty(t, name, "operation name should not be empty")
189192
assert.False(t, seen[name], "operation name should be unique: %s", name)
@@ -192,24 +195,25 @@ func TestTelemetryConstants(t *testing.T) {
192195

193196
// Verify attribute keys are well-formed
194197
attrKeys := []string{
195-
AttrGitRef,
196-
AttrGitFromRef,
197-
AttrGitToRef,
198-
AttrGitWorktreeDir,
199-
AttrGitWorkingDir,
200-
AttrGitRefCount,
201-
AttrGitDiffAdded,
202-
AttrGitDiffRemoved,
203-
AttrGitDiffChanged,
204-
AttrFilterQuery,
205-
AttrFilterType,
206-
AttrFilterCount,
207-
AttrComponentCount,
208-
AttrResultCount,
209-
AttrWorktreePairCount,
198+
filter.AttrGitRef,
199+
filter.AttrGitFromRef,
200+
filter.AttrGitToRef,
201+
filter.AttrGitWorktreeDir,
202+
filter.AttrGitWorkingDir,
203+
filter.AttrGitRefCount,
204+
filter.AttrGitDiffAdded,
205+
filter.AttrGitDiffRemoved,
206+
filter.AttrGitDiffChanged,
207+
filter.AttrFilterQuery,
208+
filter.AttrFilterType,
209+
filter.AttrFilterCount,
210+
filter.AttrComponentCount,
211+
filter.AttrResultCount,
212+
filter.AttrWorktreePairCount,
210213
}
211214

212215
seenAttrs := make(map[string]bool)
216+
213217
for _, key := range attrKeys {
214218
assert.NotEmpty(t, key, "attribute key should not be empty")
215219
assert.False(t, seenAttrs[key], "attribute key should be unique: %s", key)

‎internal/git/git.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ func (g *GitRunner) GetCurrentBranch(ctx context.Context) string {
466466
cmd := g.prepareCommand(ctx, "rev-parse", "--abbrev-ref", "HEAD")
467467

468468
var stdout bytes.Buffer
469+
469470
cmd.Stdout = &stdout
470471

471472
if err := cmd.Run(); err != nil {
@@ -484,6 +485,7 @@ func (g *GitRunner) GetHeadCommit(ctx context.Context) string {
484485
cmd := g.prepareCommand(ctx, "rev-parse", "HEAD")
485486

486487
var stdout bytes.Buffer
488+
487489
cmd.Stdout = &stdout
488490

489491
if err := cmd.Run(); err != nil {

‎internal/worktrees/worktrees.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ func NewWorktrees(
353353
if err != nil {
354354
return nil, tgerrors.Errorf("failed to create Git runner for worktree creation: %w", err)
355355
}
356+
356357
gitRunner = gitRunner.WithWorkDir(workingDir)
357358

358359
// Get repo info for telemetry
@@ -404,7 +405,9 @@ func NewWorktrees(
404405

405406
diffErr := filter.TraceGitDiff(gitCmdCtx, gitExpression.FromRef, gitExpression.ToRef, repoRemote, func(ctx context.Context) error {
406407
var err error
408+
407409
diffs, err = gitRunner.Diff(ctx, gitExpression.FromRef, gitExpression.ToRef)
410+
408411
return err
409412
})
410413
if diffErr != nil {
@@ -434,6 +437,7 @@ func NewWorktrees(
434437
gitRunner: gitRunner,
435438
}
436439
outerErr = err
440+
437441
return err
438442
}
439443

@@ -448,7 +452,7 @@ func NewWorktrees(
448452

449453
// Record telemetry for diff results
450454
if diffs := expressionsToDiffs[gitExpression]; diffs != nil {
451-
recordDiffTelemetry(ctx, gitExpression, diffs)
455+
recordDiffTelemetry(ctx, diffs)
452456
}
453457
}
454458

@@ -470,14 +474,12 @@ func NewWorktrees(
470474
}
471475

472476
// recordDiffTelemetry records telemetry metrics for git diff results.
473-
func recordDiffTelemetry(ctx context.Context, expr *filter.GitExpression, diffs *git.Diffs) {
477+
func recordDiffTelemetry(ctx context.Context, diffs *git.Diffs) {
474478
telemeter := telemetry.TelemeterFromContext(ctx)
475479
if telemeter == nil || telemeter.Meter == nil {
476480
return
477481
}
478482

479-
// Count method only takes ctx, name, and value - attributes aren't supported
480-
// Use descriptive metric names that include the context
481483
telemeter.Count(ctx, "git_diff_files_added", int64(len(diffs.Added)))
482484
telemeter.Count(ctx, "git_diff_files_removed", int64(len(diffs.Removed)))
483485
telemeter.Count(ctx, "git_diff_files_changed", int64(len(diffs.Changed)))

0 commit comments

Comments
 (0)