From 1de1e3a9a5fae7d77753bbcde698c39e1655e1b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Mon, 13 Jun 2022 12:49:57 +0200
Subject: [PATCH] Manager: add 'canary' test to all timeout checker tests

The canary test asserts that certain constants still have the expected
value. Lowering those constants is good for testing the timeout stuff with
the actual Flamenco Manager + Worker (without having to wait 5 minutes for
it to kick in), but it's too easy to accidentally run the unit tests and
get cryptic errors about everything failing horribly and miserably when
you leave those constants low.
---
 internal/manager/timeout_checker/tasks_test.go     |  9 +++------
 .../timeout_checker/timeout_checker_test.go        | 14 ++++++++++++++
 internal/manager/timeout_checker/workers_test.go   |  2 ++
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/internal/manager/timeout_checker/tasks_test.go b/internal/manager/timeout_checker/tasks_test.go
index 4334a77e..9c1227be 100644
--- a/internal/manager/timeout_checker/tasks_test.go
+++ b/internal/manager/timeout_checker/tasks_test.go
@@ -19,6 +19,8 @@ import (
 const taskTimeout = 20 * time.Minute
 
 func TestTimeoutCheckerTiming(t *testing.T) {
+	canaryTest(t)
+
 	ttc, finish, mocks := timeoutCheckerTestFixtures(t)
 	defer finish()
 
@@ -81,12 +83,7 @@ func TestTimeoutCheckerTiming(t *testing.T) {
 }
 
 func TestTaskTimeout(t *testing.T) {
-	// Canary test: if these constants do not have the expected value, the test
-	// will fail rather cryptically.
-	if !assert.Equal(t, 5*time.Minute, timeoutInitialSleep, "timeoutInitialSleep does not have the expected value") ||
-		!assert.Equal(t, 1*time.Minute, timeoutCheckInterval, "timeoutCheckInterval does not have the expected value") {
-		t.FailNow()
-	}
+	canaryTest(t)
 
 	ttc, finish, mocks := timeoutCheckerTestFixtures(t)
 	defer finish()
diff --git a/internal/manager/timeout_checker/timeout_checker_test.go b/internal/manager/timeout_checker/timeout_checker_test.go
index 82dbffae..13c33242 100644
--- a/internal/manager/timeout_checker/timeout_checker_test.go
+++ b/internal/manager/timeout_checker/timeout_checker_test.go
@@ -6,9 +6,11 @@ import (
 	"context"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/benbjohnson/clock"
 	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
 
 	"git.blender.org/flamenco/internal/manager/timeout_checker/mocks"
 )
@@ -72,3 +74,15 @@ func timeoutCheckerTestFixtures(t *testing.T) (*TimeoutChecker, func(), *Timeout
 	)
 	return sm, finish, mocks
 }
+
+// canaryTest will abort the current test if timing constants do not have the
+// expected value. Unit tests will fail rather cryptically by themselves if the
+// timing of the timeout checker is not what is expected.
+func canaryTest(t *testing.T) {
+	if assert.Equal(t, 5*time.Minute, timeoutInitialSleep, "timeoutInitialSleep does not have the expected value") &&
+		assert.Equal(t, 1*time.Minute, timeoutCheckInterval, "timeoutCheckInterval does not have the expected value") {
+		return
+	}
+	t.Fatal("timing-related constants are not as expected by the unit test, preemptively aborting.")
+	t.FailNow()
+}
diff --git a/internal/manager/timeout_checker/workers_test.go b/internal/manager/timeout_checker/workers_test.go
index 5be02249..713d1a3d 100644
--- a/internal/manager/timeout_checker/workers_test.go
+++ b/internal/manager/timeout_checker/workers_test.go
@@ -15,6 +15,8 @@ import (
 const workerTimeout = 20 * time.Minute
 
 func TestWorkerTimeout(t *testing.T) {
+	canaryTest(t)
+
 	ttc, finish, mocks := timeoutCheckerTestFixtures(t)
 	defer finish()