Skip to content

Commit 03c9205

Browse files
indacoindaco
authored
refactor: consolidate test utilities, add channel buffer logging and worker validation (#102)
* refactor(testutils): consolidate testhelpers into testutils * fix(sync): log warning when channel buffers are full * fix(worker): panic on invalid NumWorkers in NewWorkerPoolManager
1 parent 78a322f commit 03c9205

File tree

21 files changed

+195
-170
lines changed

21 files changed

+195
-170
lines changed

‎cmd/tempo/componentcmd/define_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/indaco/tempo/internal/app"
1111
"github.com/indaco/tempo/internal/config"
1212
"github.com/indaco/tempo/internal/logger"
13-
"github.com/indaco/tempo/internal/testhelpers"
1413
"github.com/indaco/tempo/internal/testutils"
1514
"github.com/indaco/tempo/internal/utils"
1615
"github.com/urfave/cli/v3"
@@ -57,7 +56,7 @@ func TestComponentCommand_DefineSubComd(t *testing.T) {
5756
t.Fatalf("Failed to capture stdout: %v", err)
5857
}
5958

60-
testhelpers.ValidateCLIOutput(t, output, []string{
59+
testutils.ValidateCLIOutput(t, output, []string{
6160
"✔ Templates for the component and assets (CSS and JS) have been created",
6261
"✔ Tempo action file for 'component' has been created",
6362
})
@@ -175,7 +174,7 @@ func TestComponentCommand_DefineSubComd_AlreadyExists(t *testing.T) {
175174
t.Fatalf("Failed to capture stdout: %v", err)
176175
}
177176

178-
testhelpers.ValidateCLIOutput(t, output, []string{
177+
testutils.ValidateCLIOutput(t, output, []string{
179178
"✔ Templates for the component and assets (CSS and JS) have been created",
180179
"✔ Tempo action file for 'component' has been created",
181180
})
@@ -195,7 +194,7 @@ func TestComponentCommand_DefineSubComd_AlreadyExists(t *testing.T) {
195194
t.Fatalf("Failed to capture stdout: %v", err)
196195
}
197196

198-
testhelpers.ValidateCLIOutput(t, output, []string{
197+
testutils.ValidateCLIOutput(t, output, []string{
199198
"⚠ Templates for 'component' already exist.",
200199
" Use '--force' to overwrite them. Any changes will be lost.",
201200
" - path: " + cfg.Paths.TemplatesDir + "/component",

‎cmd/tempo/componentcmd/new_test.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/indaco/tempo/internal/config"
1313
"github.com/indaco/tempo/internal/logger"
1414
"github.com/indaco/tempo/internal/templatefuncs/providers/gonameprovider"
15-
"github.com/indaco/tempo/internal/testhelpers"
1615
"github.com/indaco/tempo/internal/testutils"
1716
"github.com/indaco/tempo/internal/utils"
1817
"github.com/urfave/cli/v3"
@@ -64,7 +63,7 @@ func TestComponentCommand_NewSubCmd_DefaultConfig(t *testing.T) {
6463

6564
// Step 2: Run "component new" to test the command
6665
t.Run("Component with default config", func(t *testing.T) {
67-
output, err := testhelpers.CaptureStdout(func() {
66+
output, err := testutils.CaptureStdout(func() {
6867
args := []string{
6968
"tempo", "component", "new",
7069
"--name", "button",
@@ -78,7 +77,7 @@ func TestComponentCommand_NewSubCmd_DefaultConfig(t *testing.T) {
7877
t.Fatalf("Failed to capture stdout: %v", err)
7978
}
8079

81-
testhelpers.ValidateCLIOutput(t, output, []string{
80+
testutils.ValidateCLIOutput(t, output, []string{
8281
"✔ Templ component files have been created",
8382
})
8483

@@ -129,7 +128,7 @@ func TestComponentCommand_NewSubCmd_WithFlags(t *testing.T) {
129128

130129
// Step 2: Run "component new" to test the command
131130
t.Run("Component with configs by flags", func(t *testing.T) {
132-
output, err := testhelpers.CaptureStdout(func() {
131+
output, err := testutils.CaptureStdout(func() {
133132
args := []string{
134133
"tempo",
135134
"component",
@@ -147,7 +146,7 @@ func TestComponentCommand_NewSubCmd_WithFlags(t *testing.T) {
147146
t.Fatalf("Failed to capture stdout: %v", err)
148147
}
149148

150-
testhelpers.ValidateCLIOutput(t, output, []string{
149+
testutils.ValidateCLIOutput(t, output, []string{
151150
"✔ Templ component files have been created",
152151
})
153152

@@ -202,7 +201,7 @@ func TestComponentCommand_NewSubComd_DryRun(t *testing.T) {
202201

203202
// Run the "component new" command with --dry-run flag.
204203
t.Run("Component Dry Run", func(t *testing.T) {
205-
output, err := testhelpers.CaptureStdout(func() {
204+
output, err := testutils.CaptureStdout(func() {
206205
args := []string{
207206
"tempo", "component", "new",
208207
"--name", "dryrun",
@@ -272,7 +271,7 @@ func TestComponentCommand_NewSubCmd_DryRun_NoChanges(t *testing.T) {
272271
"--dry-run",
273272
}
274273

275-
output, err := testhelpers.CaptureStdout(func() {
274+
output, err := testutils.CaptureStdout(func() {
276275
err := cliApp.Run(context.Background(), args)
277276
if err != nil {
278277
t.Fatalf("Command failed: %v", err)
@@ -482,7 +481,7 @@ func TestComponentCommand_NewSubCmd_CheckComponentExists(t *testing.T) {
482481

483482
// Step 2: Run "component new" to test the command
484483
t.Run("Component with default config", func(t *testing.T) {
485-
output, err := testhelpers.CaptureStdout(func() {
484+
output, err := testutils.CaptureStdout(func() {
486485
args := []string{
487486
"tempo", "component", "new",
488487
"--name", "button",
@@ -496,7 +495,7 @@ func TestComponentCommand_NewSubCmd_CheckComponentExists(t *testing.T) {
496495
t.Fatalf("Failed to capture stdout: %v", err)
497496
}
498497

499-
testhelpers.ValidateCLIOutput(t, output, []string{
498+
testutils.ValidateCLIOutput(t, output, []string{
500499
"✔ Templ component files have been created",
501500
})
502501

@@ -510,7 +509,7 @@ func TestComponentCommand_NewSubCmd_CheckComponentExists(t *testing.T) {
510509

511510
// Step 3: Run "component new" again
512511
t.Run("Fail Component Creation When The Same already exists", func(t *testing.T) {
513-
output, err := testhelpers.CaptureStdout(func() {
512+
output, err := testutils.CaptureStdout(func() {
514513
args := []string{
515514
"tempo", "component", "new",
516515
"--name", "button",

‎cmd/tempo/initcmd/init_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/indaco/tempo/internal/app"
1212
"github.com/indaco/tempo/internal/config"
1313
"github.com/indaco/tempo/internal/logger"
14-
"github.com/indaco/tempo/internal/testhelpers"
1514
"github.com/indaco/tempo/internal/testutils"
1615
"github.com/indaco/tempo/internal/utils"
1716
"github.com/urfave/cli/v3"
@@ -59,7 +58,7 @@ func TestInitCommand(t *testing.T) {
5958
SetupInitCommand(cliCtx),
6059
}
6160

62-
output, err := testhelpers.CaptureStdout(func() {
61+
output, err := testutils.CaptureStdout(func() {
6362
args := []string{"tempo", "init", "--base-folder", baseFolder}
6463
err := cliApp.Run(context.Background(), args)
6564
if err != nil {
@@ -70,7 +69,7 @@ func TestInitCommand(t *testing.T) {
7069
t.Fatalf("Failed to capture stdout: %v", err)
7170
}
7271

73-
testhelpers.ValidateCLIOutput(t, output, []string{
72+
testutils.ValidateCLIOutput(t, output, []string{
7473
"ℹ Generating",
7574
"✔ Done! Customize it to match your project needs.",
7675
})

‎cmd/tempo/main_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/indaco/tempo/internal/app"
1212
"github.com/indaco/tempo/internal/config"
1313
"github.com/indaco/tempo/internal/logger"
14-
"github.com/indaco/tempo/internal/testhelpers"
14+
"github.com/indaco/tempo/internal/testutils"
1515
"github.com/indaco/tempo/internal/utils"
1616
"github.com/indaco/tempo/internal/version"
1717
)
@@ -138,15 +138,15 @@ func TestRunApp_Help(t *testing.T) {
138138
}
139139
os.Stdout = w
140140

141-
output, err := testhelpers.CaptureStdout(func() {
141+
output, err := testutils.CaptureStdout(func() {
142142
if err := runCLI(args); err != nil {
143143
t.Fatalf("runApp returned error: %v", err)
144144
}
145145
})
146146
if err != nil {
147147
t.Fatalf("Failed to capture output: %v", err)
148148
}
149-
testhelpers.ValidateCLIOutput(t, output, []string{"USAGE:", "COMMANDS:"})
149+
testutils.ValidateCLIOutput(t, output, []string{"USAGE:", "COMMANDS:"})
150150
}
151151

152152
// TestRunApp_InitAutoGen verifies that when no config file exists, the "init" command auto-generates one.
@@ -161,7 +161,7 @@ func TestRunApp_InitAutoGen(t *testing.T) {
161161
}
162162

163163
// Run the command and capture output
164-
output, err := testhelpers.CaptureStdout(func() {
164+
output, err := testutils.CaptureStdout(func() {
165165
_ = runCLI([]string{"tempo", "init", "--base-folder", tempDir})
166166
})
167167

@@ -175,7 +175,7 @@ func TestRunApp_InitAutoGen(t *testing.T) {
175175
}
176176

177177
// Validate output messages
178-
testhelpers.ValidateCLIOutput(t, output, []string{"Generating", "Done!"})
178+
testutils.ValidateCLIOutput(t, output, []string{"Generating", "Done!"})
179179
}
180180

181181
/* ------------------------------------------------------------------------- */

‎cmd/tempo/registercmd/register_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/indaco/tempo/internal/config"
1111
"github.com/indaco/tempo/internal/logger"
1212
"github.com/indaco/tempo/internal/templatefuncs/registry"
13-
"github.com/indaco/tempo/internal/testhelpers"
1413
"github.com/indaco/tempo/internal/testutils"
1514
"github.com/indaco/tempo/internal/utils"
1615
)
@@ -72,7 +71,7 @@ func TestRegisterCommand_Functions_Repo(t *testing.T) {
7271
"--url", "https://github.com/indaco/tempo-provider-sprig.git",
7372
}
7473

75-
output, err := testhelpers.CaptureStdout(func() {
74+
output, err := testutils.CaptureStdout(func() {
7675
if err := cmd.Run(context.Background(), args); err != nil {
7776
t.Fatalf("Command failed: %v", err)
7877
}
@@ -157,7 +156,7 @@ var Provider templatefuncs.TemplateFuncProvider = &MockProvider{}
157156
"--path", providerDir,
158157
}
159158

160-
output, err := testhelpers.CaptureStdout(func() {
159+
output, err := testutils.CaptureStdout(func() {
161160
if err := cmd.Run(context.Background(), args); err != nil {
162161
t.Fatalf("Command failed: %v", err)
163162
}
@@ -238,7 +237,7 @@ var Provider templatefuncs.TemplateFuncProvider = &MockProvider{}
238237
"--path", providerDir,
239238
}
240239

241-
output, err := testhelpers.CaptureStdout(func() {
240+
output, err := testutils.CaptureStdout(func() {
242241
if err := cmd.Run(context.Background(), args); err != nil {
243242
t.Fatalf("Command failed: %v", err)
244243
}

‎cmd/tempo/synccmd/sync.go‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func runWorkerPool(
208208
})
209209

210210
// Queue files for processing before closing job channel & starting workers
211-
if err := queueFilesForProcessing(opts, manager, lastRunTimestamp); err != nil {
211+
if err := queueFilesForProcessing(cmdCtx.Logger, opts, manager, lastRunTimestamp); err != nil {
212212
return apperrors.Wrap("Failed to queue files", err)
213213
}
214214

@@ -251,24 +251,25 @@ func runWorkerPool(
251251

252252
// queueFilesForProcessing walks through the input directory and enqueues jobs.
253253
func queueFilesForProcessing(
254+
log logger.Logger,
254255
opts worker.WorkerPoolOptions,
255256
manager *worker.WorkerPoolManager,
256257
lastRunTimestamp int64,
257258
) error {
258259
return filepath.WalkDir(opts.InputDir, func(source string, d os.DirEntry, err error) error {
259260
if err != nil {
260-
handleError(manager, source, err)
261+
handleError(log, manager, source, err)
261262
return nil
262263
}
263264

264265
absPath, err := filepath.Abs(source)
265266
if err != nil {
266-
handleError(manager, source, err)
267+
handleError(log, manager, source, err)
267268
return nil
268269
}
269270

270271
if shouldExcludeDir(opts.ExcludeDir, absPath) || isExcludedFile(absPath) {
271-
handleSkip(manager.SkippedChan, worker.SkippedFile{
272+
handleSkip(log, manager.SkippedChan, worker.SkippedFile{
272273
Source: source,
273274
Dest: "", // No expected output file
274275
InputDir: opts.InputDir,
@@ -280,9 +281,9 @@ func queueFilesForProcessing(
280281
}
281282

282283
outputFilePath := utils.RebasePathToOutput(source, opts.InputDir, opts.OutputDir)
283-
if !d.IsDir() && shouldProcessFile(source, outputFilePath, opts, lastRunTimestamp, manager) {
284+
if !d.IsDir() && shouldProcessFile(log, source, outputFilePath, opts, lastRunTimestamp, manager) {
284285
if !enqueueJob(manager, source, outputFilePath) {
285-
handleSkip(manager.SkippedChan, worker.SkippedFile{
286+
handleSkip(log, manager.SkippedChan, worker.SkippedFile{
286287
Source: source,
287288
Dest: outputFilePath,
288289
InputDir: opts.InputDir,
@@ -407,26 +408,27 @@ func handleSummary(
407408

408409
// handleError sends errors to the error channel.
409410
// With large buffer sizes (numWorkers * 100), blocking is unlikely.
410-
// Uses non-blocking send as a safety fallback.
411-
func handleError(manager *worker.WorkerPoolManager, path string, err error) {
411+
// Uses non-blocking send as a safety fallback; logs a warning if the buffer is full.
412+
func handleError(log logger.Logger, manager *worker.WorkerPoolManager, path string, err error) {
412413
select {
413414
case manager.ErrorsChan <- worker.FormatError(path, err):
414415
// Successfully sent
415416
default:
416-
// Buffer full - extremely unlikely with large buffers
417-
// Log would go here in production, but we avoid blocking
417+
// Buffer full - log so the error is not silently lost
418+
log.Warning("error channel buffer full, dropping error for path: %s (%v)", path, err)
418419
}
419420
}
420421

421422
// handleSkip sends skip reasons to the skipped channel.
422423
// With large buffer sizes (numWorkers * 100), blocking is unlikely.
423-
// Uses non-blocking send as a safety fallback.
424-
func handleSkip(ch chan<- worker.ProcessingError, skipped worker.SkippedFile) {
424+
// Uses non-blocking send as a safety fallback; logs a warning if the buffer is full.
425+
func handleSkip(log logger.Logger, ch chan<- worker.ProcessingError, skipped worker.SkippedFile) {
425426
select {
426427
case ch <- worker.FormatSkipReason(skipped):
427428
// Successfully sent
428429
default:
429-
// Buffer full - extremely unlikely with large buffers
430+
// Buffer full - log so the skip is not silently lost
431+
log.Warning("skipped channel buffer full, dropping skip for path: %s", skipped.Source)
430432
}
431433
}
432434

@@ -436,9 +438,9 @@ func shouldExcludeDir(excludeDir, absPath string) bool {
436438
}
437439

438440
// shouldProcessFile decides whether the file should be processed or skipped.
439-
func shouldProcessFile(source, dest string, opts worker.WorkerPoolOptions, lastRunTimestamp int64, manager *worker.WorkerPoolManager) bool {
441+
func shouldProcessFile(log logger.Logger, source, dest string, opts worker.WorkerPoolOptions, lastRunTimestamp int64, manager *worker.WorkerPoolManager) bool {
440442
if isExcludedFile(source) {
441-
handleSkip(manager.SkippedChan, worker.SkippedFile{
443+
handleSkip(log, manager.SkippedChan, worker.SkippedFile{
442444
Source: source,
443445
Dest: dest,
444446
InputDir: opts.InputDir,
@@ -451,12 +453,12 @@ func shouldProcessFile(source, dest string, opts worker.WorkerPoolOptions, lastR
451453

452454
lastModified, err := getFileLastModifiedTime(source)
453455
if err != nil {
454-
handleError(manager, source, err)
456+
handleError(log, manager, source, err)
455457
return false
456458
}
457459

458460
if !opts.IsProduction && !opts.IsForce && lastModified < lastRunTimestamp {
459-
handleSkip(manager.SkippedChan, worker.SkippedFile{
461+
handleSkip(log, manager.SkippedChan, worker.SkippedFile{
460462
Source: source,
461463
Dest: dest,
462464
InputDir: opts.InputDir,

0 commit comments

Comments
 (0)