Fix incorrect processing of VerificationFreq parameter (livepeer#2740)

* fix semantics of shouldSkipVerification

* go fmt

* Fix test to align with the meaning of verificationFreq parameter

server/broadcast.go

@@ -397,7 +397,7 @@ func (bsm *BroadcastSessionsManager) shouldSkipVerification(sessions []*Broadcas
 	if !includesSession(sessions, bsm.verifiedSession) {
 		return false
 	}
-	return common.RandomUintUnder(bsm.VerificationFreq) == 0
+	return common.RandomUintUnder(bsm.VerificationFreq) != 0
 }
 
 func NewSessionManager(ctx context.Context, node *core.LivepeerNode, params *core.StreamParameters, sel BroadcastSessionsSelectorFactory) *BroadcastSessionsManager {

server/broadcast_test.go

@@ -1796,8 +1796,9 @@ func TestVerifcationDoesntRunWhenNoVerifiedSessionPresent(t *testing.T) {
 }
 
 func TestVerifcationRunsBasedOnVerificationFrequency(t *testing.T) {
+	verificationFreq := 5
 	b := BroadcastSessionsManager{
-		VerificationFreq: 5, // Run approximately 1 in 5 times
+		VerificationFreq: uint(verificationFreq), // Verification should run approximately 1 in 5 times
 	}
 
 	var verifiedSession = &BroadcastSession{
@@ -1806,15 +1807,14 @@ func TestVerifcationRunsBasedOnVerificationFrequency(t *testing.T) {
 
 	b.verifiedSession = verifiedSession
 
-	var shouldRunCount int
-	for i := 0; i < 10000; i++ {
+	var shouldSkipCount int
+	numTests := 10000
+	for i := 0; i < numTests; i++ {
 		if b.shouldSkipVerification([]*BroadcastSession{verifiedSession}) {
-			shouldRunCount++
+			shouldSkipCount++
 		}
 	}
 
-	// Difficult to test a random function, so we just check that it's in
-	// the ball park of what we'd expect (1 in 5 of 10,000 tries)
-	require.Greater(t, shouldRunCount, 1000)
-	require.Less(t, shouldRunCount, 3000)
+	require.Greater(t, float32(shouldSkipCount), float32(numTests)*(1-2/float32(verificationFreq)))
+	require.Less(t, float32(shouldSkipCount), float32(numTests)*(1-0.5/float32(verificationFreq)))
 }

server/push_test.go

@@ -1657,7 +1657,7 @@ func TestPush_MultipartReturnMultiSession(t *testing.T) {
 	sess3.OrchestratorScore = common.Score_Untrusted
 
 	bsm := bsmWithSessListExt([]*BroadcastSession{sess1}, []*BroadcastSession{sess3, sess2}, false)
-	bsm.VerificationFreq = math.MaxInt
+	bsm.VerificationFreq = 1
 	assert.Equal(0, bsm.untrustedPool.sus.count)
 	// hack: stop pool from refreshing
 	bsm.untrustedPool.refreshing = true