From 5dc000432791cd81070a25a483e4f79985dfb460 Mon Sep 17 00:00:00 2001
From: Blake Gentry <blakesgentry@gmail.com>
Date: Thu, 29 Aug 2024 21:42:52 -0500
Subject: [PATCH] add client-level test coverage for PeriodicJobConstructor nil
 return

---
 CHANGELOG.md         |  5 +++--
 client_test.go       | 27 +++++++++++++++++++++++++++
 periodic_job_test.go |  2 +-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 97c71423..9cd52e19 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,10 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
   ```go
   # before
-	migrator := rivermigrate.New(riverpgxv5.New(dbPool), nil)
+  migrator := rivermigrate.New(riverpgxv5.New(dbPool), nil)
 
   # after
-	migrator, err := rivermigrate.New(riverpgxv5.New(dbPool), nil)
+  migrator, err := rivermigrate.New(riverpgxv5.New(dbPool), nil)
   if err != nil {
       // handle error
   }
@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Fixed
 
 - Fixed a panic that'd occur if `StopAndCancel` was invoked before a client was started. [PR #557](https://github.com/riverqueue/river/pull/557).
+- A `PeriodicJobConstructor` should be able to return `nil` `JobArgs` if it wishes to not have any job inserted. However, this was either never working or was broken at some point. It's now fixed. Thanks [@semanser](https://github.com/semanser)! [PR #567](https://github.com/riverqueue/river/pull/567).
 
 ## [0.11.4] - 2024-08-20
 
diff --git a/client_test.go b/client_test.go
index f118e435..503ed71f 100644
--- a/client_test.go
+++ b/client_test.go
@@ -2727,6 +2727,33 @@ func Test_Client_Maintenance(t *testing.T) {
 		require.Empty(t, jobs)
 	})
 
+	t.Run("PeriodicJobConstructorReturningNil", func(t *testing.T) {
+		t.Parallel()
+
+		config := newTestConfig(t, nil)
+
+		worker := &periodicJobWorker{}
+		AddWorker(config.Workers, worker)
+		config.PeriodicJobs = []*PeriodicJob{
+			NewPeriodicJob(cron.Every(15*time.Minute), func() (JobArgs, *InsertOpts) {
+				// Returning nil from the constructor function should not insert a new
+				// job and should be handled cleanly
+				return nil, nil
+			}, &PeriodicJobOpts{RunOnStart: true}),
+		}
+
+		client, bundle := setup(t, config)
+
+		startAndWaitForQueueMaintainer(ctx, t, client)
+
+		svc := maintenance.GetService[*maintenance.PeriodicJobEnqueuer](client.queueMaintainer)
+		svc.TestSignals.SkippedJob.WaitOrTimeout()
+
+		jobs, err := bundle.exec.JobGetByKindMany(ctx, []string{(periodicJobArgs{}).Kind()})
+		require.NoError(t, err)
+		require.Len(t, jobs, 0, "Expected to find zero jobs of kind: "+(periodicJobArgs{}).Kind())
+	})
+
 	t.Run("PeriodicJobEnqueuerAddDynamically", func(t *testing.T) {
 		t.Parallel()
 
diff --git a/periodic_job_test.go b/periodic_job_test.go
index ae469e04..745a06d8 100644
--- a/periodic_job_test.go
+++ b/periodic_job_test.go
@@ -16,7 +16,7 @@ func TestPeriodicJobBundle(t *testing.T) {
 
 	type testBundle struct{}
 
-	setup := func(t *testing.T) (*PeriodicJobBundle, *testBundle) {
+	setup := func(t *testing.T) (*PeriodicJobBundle, *testBundle) { //nolint:unparam
 		t.Helper()
 
 		periodicJobEnqueuer := maintenance.NewPeriodicJobEnqueuer(