Skip to content

Commit

Permalink
Add review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
bjee19 committed Feb 6, 2024
1 parent 9ca0236 commit de6e3bc
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 15 deletions.
13 changes: 2 additions & 11 deletions internal/framework/runnables/cronjob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,15 @@ func TestCronJob_ContextCanceled(t *testing.T) {

readyChannel := make(chan struct{})

timeout := 10 * time.Second
var callCount int

valCh := make(chan int, 128)
worker := func(context.Context) {
callCount++
valCh <- callCount
}

cfg := CronJobConfig{
Worker: worker,
Worker: func(ctx context.Context) {},
Logger: zap.New(),
Period: 1 * time.Millisecond, // 1ms is much smaller than timeout so the CronJob should run a few times
ReadyCh: readyChannel,
}
job := NewCronJob(cfg)

ctx, cancel := context.WithTimeout(context.Background(), timeout)
ctx, cancel := context.WithCancel(context.Background())

errCh := make(chan error)
go func() {
Expand Down
6 changes: 3 additions & 3 deletions internal/mode/static/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func StartManager(cfg config.Config) error {
ConfigurationGetter: eventHandler,
Version: cfg.Version,
})
if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker)); err != nil {
if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil {
return fmt.Errorf("cannot register telemetry job: %w", err)
}

Expand Down Expand Up @@ -408,7 +408,7 @@ func registerControllers(
func createTelemetryJob(
cfg config.Config,
dataCollector telemetry.DataCollector,
nginxChecker *nginxConfiguredOnStartChecker,
readyCh <-chan struct{},
) *runnables.Leader {
logger := cfg.Logger.WithName("telemetryJob")
exporter := telemetry.NewLoggingExporter(cfg.Logger.WithName("telemetryExporter").V(1 /* debug */))
Expand All @@ -426,7 +426,7 @@ func createTelemetryJob(
Logger: logger,
Period: cfg.TelemetryReportPeriod,
JitterFactor: jitterFactor,
ReadyCh: nginxChecker.getReadyCh(),
ReadyCh: readyCh,
},
),
}
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/telemetry/job_worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ func TestCreateTelemetryJobWorker(t *testing.T) {

timeout := 10 * time.Second
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

worker(ctx)
_, data := exporter.ExportArgsForCall(0)
g.Expect(data).To(Equal(expData))
cancel()
}

0 comments on commit de6e3bc

Please sign in to comment.