Repository: nats-io/nats-server
Pull request #1770 (2020-12-11 22:32:24)

Break test up into shorter ones
Signed-off-by: Matthias Hanel <mh@synadia.com>

 each test takes about 2 seconds. one second to observe expiration does not happen, and one to observe expiration.

kozlovic: We could maybe use UnixNano() in dirstore.go which would help for tests that don't rely on JWT's expiration time to have a shorter running time. I have sent you some proposed changes and you will have a look and if that works, you will update this PR.

Patch:
diff --git a/server/dirstore.go b/server/dirstore.go
index 663aabef..c1fb8b71 100644
--- a/server/dirstore.go
+++ b/server/dirstore.go
@@ -607,7 +607,7 @@ func (pq *expirationTracker) updateTrack(publicKey string) {
 		i := e.Value.(*jwtItem)
 		if pq.ttl != 0 {
 			// only update expiration when set
-			i.expiration = time.Now().Round(time.Second).Add(pq.ttl).Unix()
+			i.expiration = time.Now().Add(pq.ttl).UnixNano()
 			heap.Fix(pq, i.index)
 		}
 		if pq.evictOnLimit {
@@ -631,11 +631,11 @@ func (pq *expirationTracker) track(publicKey string, hash *[sha256.Size]byte, th
 		if pq.ttl == time.Duration(math.MaxInt64) {
 			exp = math.MaxInt64
 		} else {
-			exp = time.Now().Round(time.Second).Add(pq.ttl).Unix()
+			exp = time.Now().Add(pq.ttl).UnixNano()
 		}
 	} else {
 		if g, err := jwt.DecodeGeneric(theJWT); err == nil {
-			exp = g.Expires
+			exp = time.Unix(g.Expires, 0).UnixNano()
 		}
 		if exp == 0 {
 			exp = math.MaxInt64 // default to indefinite
@@ -683,10 +683,10 @@ func (store *DirJWTStore) startExpiring(reCheck time.Duration, limit int64, evic
 		defer t.Stop()
 		defer pq.wg.Done()
 		for {
-			now := time.Now()
+			now := time.Now().UnixNano()
 			store.Lock()
 			if pq.Len() > 0 {
-				if it := pq.heap[0]; it.expiration <= now.Unix() {
+				if it := pq.heap[0]; it.expiration <= now {
 					path := store.pathForKey(it.publicKey)
 					if err := os.Remove(path); err == nil {
 						heap.Pop(pq)


Test patch:
diff --git a/server/dirstore_test.go b/server/dirstore_test.go
index 7e81a740..e47dfef8 100644
--- a/server/dirstore_test.go
+++ b/server/dirstore_test.go
@@ -83,6 +83,7 @@ func require_Len(t *testing.T, a, b int) {
 }
 
 func TestShardedDirStoreWriteAndReadonly(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -136,6 +137,7 @@ func TestShardedDirStoreWriteAndReadonly(t *testing.T) {
 }
 
 func TestUnshardedDirStoreWriteAndReadonly(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -191,11 +193,13 @@ func TestUnshardedDirStoreWriteAndReadonly(t *testing.T) {
 }
 
 func TestNoCreateRequiresDir(t *testing.T) {
+	t.Parallel()
 	_, err := NewDirJWTStore("/a/b/c", true, false)
 	require_Error(t, err)
 }
 
 func TestCreateMakesDir(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -214,6 +218,7 @@ func TestCreateMakesDir(t *testing.T) {
 }
 
 func TestShardedDirStorePackMerge(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 	dir2, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
@@ -291,6 +296,7 @@ func TestShardedDirStorePackMerge(t *testing.T) {
 }
 
 func TestShardedToUnsharedDirStorePackMerge(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 	dir2, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
@@ -351,6 +357,7 @@ func TestShardedToUnsharedDirStorePackMerge(t *testing.T) {
 }
 
 func TestMergeOnlyOnNewer(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -424,6 +431,7 @@ func assertStoreSize(t *testing.T, dirStore *DirJWTStore, length int) {
 }
 
 func TestExpiration(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -461,6 +469,7 @@ func TestExpiration(t *testing.T) {
 }
 
 func TestLimit(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -503,6 +512,7 @@ func TestLimit(t *testing.T) {
 }
 
 func TestLimitNoEvict(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -555,6 +565,7 @@ func TestLimitNoEvict(t *testing.T) {
 }
 
 func TestLruLoad(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 	dirStore, err := NewExpiringDirJWTStore(dir, false, false, NoDelete, time.Millisecond*100, 2, true, 0, nil)
@@ -587,6 +598,7 @@ func TestLruLoad(t *testing.T) {
 }
 
 func TestLruVolume(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -629,6 +641,7 @@ func TestLruVolume(t *testing.T) {
 }
 
 func TestLru(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -678,6 +691,7 @@ func TestLru(t *testing.T) {
 }
 
 func TestReload(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 	notificationChan := make(chan struct{}, 5)
@@ -739,6 +753,7 @@ func TestReload(t *testing.T) {
 }
 
 func TestExpirationUpdate(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 
@@ -756,17 +771,17 @@ func TestExpirationUpdate(t *testing.T) {
 	require_NotEqual(t, h, nh)
 	h = nh
 
-	time.Sleep(2200 * time.Millisecond)
+	time.Sleep(1500 * time.Millisecond)
 	f, err := ioutil.ReadDir(dir)
 	require_NoError(t, err)
 	require_Len(t, len(f), 1)
 
-	createTestAccount(t, dirStore, 5, accountKey)
+	createTestAccount(t, dirStore, 2, accountKey)
 	nh = dirStore.Hash()
 	require_NotEqual(t, h, nh)
 	h = nh
 
-	time.Sleep(2200 * time.Millisecond)
+	time.Sleep(1500 * time.Millisecond)
 	f, err = ioutil.ReadDir(dir)
 	require_NoError(t, err)
 	require_Len(t, len(f), 1)
@@ -776,7 +791,7 @@ func TestExpirationUpdate(t *testing.T) {
 	require_NotEqual(t, h, nh)
 	h = nh
 
-	time.Sleep(2200 * time.Millisecond)
+	time.Sleep(1500 * time.Millisecond)
 	f, err = ioutil.ReadDir(dir)
 	require_NoError(t, err)
 	require_Len(t, len(f), 1)
@@ -786,7 +801,7 @@ func TestExpirationUpdate(t *testing.T) {
 	require_NotEqual(t, h, nh)
 	h = nh
 
-	time.Sleep(2200 * time.Millisecond)
+	time.Sleep(1500 * time.Millisecond)
 	f, err = ioutil.ReadDir(dir)
 	require_NoError(t, err)
 	require_Len(t, len(f), 0)
@@ -797,6 +812,7 @@ func TestExpirationUpdate(t *testing.T) {
 }
 
 func TestTTL(t *testing.T) {
+	t.Parallel()
 	dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 	require_NoError(t, err)
 	require_OneJWT := func() {
@@ -805,35 +821,49 @@ func TestTTL(t *testing.T) {
 		require_NoError(t, err)
 		require_Len(t, len(f), 1)
 	}
-	dirStore, err := NewExpiringDirJWTStore(dir, false, false, NoDelete, 50*time.Millisecond, 10, true, time.Second, nil)
-	require_NoError(t, err)
-	defer dirStore.Close()
+	test := func(op func(store *DirJWTStore, accountKey nkeys.KeyPair, accountPubKey string, jwt string)) {
+		dirStore, err := NewExpiringDirJWTStore(dir, false, false, NoDelete, 50*time.Millisecond, 10, true, 200*time.Millisecond, nil)
+		require_NoError(t, err)
+		defer dirStore.Close()
 
-	accountKey, err := nkeys.CreateAccount()
-	require_NoError(t, err)
-	pubKey, err := accountKey.PublicKey()
-	require_NoError(t, err)
-	jwt := createTestAccount(t, dirStore, 0, accountKey)
-	require_OneJWT()
-	for i := 0; i < 3; i++ {
-		time.Sleep(250 * time.Millisecond)
-		dirStore.LoadAcc(pubKey)
-		require_OneJWT()
-	}
-	for i := 0; i < 3; i++ {
-		time.Sleep(250 * time.Millisecond)
-		dirStore.SaveAcc(pubKey, jwt)
-		require_OneJWT()
-	}
-	for i := 0; i < 3; i++ {
-		time.Sleep(250 * time.Millisecond)
-		createTestAccount(t, dirStore, 0, accountKey)
+		accountKey, err := nkeys.CreateAccount()
+		require_NoError(t, err)
+		pubKey, err := accountKey.PublicKey()
+		require_NoError(t, err)
+		jwt := createTestAccount(t, dirStore, 0, accountKey)
 		require_OneJWT()
+		// observe non expiration due to activity
+		for i := 0; i < 4; i++ {
+			time.Sleep(110 * time.Millisecond)
+			op(dirStore, accountKey, pubKey, jwt)
+			require_OneJWT()
+		}
+		// observe expiration
+		for i := 0; i < 40; i++ {
+			time.Sleep(50 * time.Millisecond)
+			f, err := ioutil.ReadDir(dir)
+			require_NoError(t, err)
+			if len(f) == 0 {
+				return
+			}
+		}
+		t.Fatalf("jwt should have expired by now")
 	}
-	time.Sleep(2 * time.Second)
-	f, err := ioutil.ReadDir(dir)
-	require_NoError(t, err)
-	require_Len(t, len(f), 0)
+	t.Run("no expiration due to load", func(t *testing.T) {
+		test(func(store *DirJWTStore, accountKey nkeys.KeyPair, pubKey string, jwt string) {
+			store.LoadAcc(pubKey)
+		})
+	})
+	t.Run("no expiration due to store", func(t *testing.T) {
+		test(func(store *DirJWTStore, accountKey nkeys.KeyPair, pubKey string, jwt string) {
+			store.SaveAcc(pubKey, jwt)
+		})
+	})
+	t.Run("no expiration due to overwrite", func(t *testing.T) {
+		test(func(store *DirJWTStore, accountKey nkeys.KeyPair, pubKey string, jwt string) {
+			createTestAccount(t, store, 0, accountKey)
+		})
+	})
 }
 
 func TestRemove(t *testing.T) {
@@ -846,6 +876,7 @@ func TestRemove(t *testing.T) {
 		NoDelete:      {1, 0},
 	} {
 		t.Run("", func(t *testing.T) {
+			t.Parallel()
 			dir, err := ioutil.TempDir(os.TempDir(), "jwtstore_test")
 			require_NoError(t, err)
 			require_OneJWT := func() {
@@ -877,6 +908,7 @@ func TestRemove(t *testing.T) {
 const infDur = time.Duration(math.MaxInt64)
 
 func TestNotificationOnPack(t *testing.T) {
+	t.Parallel()
 	jwts := map[string]string{
 		"key1": "value",
 		"key2": "value",
@@ -941,6 +973,7 @@ func TestNotificationOnPack(t *testing.T) {
 }
 
 func TestNotificationOnPackWalk(t *testing.T) {
+	t.Parallel()
 	const storeCnt = 5
 	const keyCnt = 50
 	const iterCnt = 8