Repository: hashicorp/nomad
File: nomad/encrypter_test.go (Go)

Code before:
	// check no cipher response was sent.
	err = encrypter.decryptWrappedKeyTask(ctx, kmsWrapper, key.Meta, wrappedKey, respCh)
	must.ErrorContains(t, err, "operation cancelled")
	must.Eq(t, 0, len(respCh))

	// Recreate the response channel so that it is no longer buffered. The
	// decrypt task should now block on attempting to send to it.
	respCh = make(chan *cipherSet)

	// Generate a new context and an error channel so we can gather the response
	// of decryptWrappedKeyTask running inside a goroutine.
	ctx, cancel = context.WithCancel(context.Background())

	errorCh := make(chan error, 1)

	// Launch the decryptWrappedKeyTask routine.
	go func() {
		err := encrypter.decryptWrappedKeyTask(ctx, kmsWrapper, key.Meta, wrappedKey, respCh)
		errorCh <- err
	}()

	// Roughly ensure the decrypt task is running for enough time to get past
	// the cipher generation.
	//
	// The routine should reach a point where it is waiting to send the cipher
	// on the response channel or for the context to be canceled.
	//
	// Canceling the context should cause the routine to exit and send an error
	// which we can check to ensure we correctly unblock.
	timer, timerStop := helper.NewSafeTimer(500 * time.Millisecond)
	defer timerStop()

	<-timer.C
	cancel()

	timer, timerStop = helper.NewSafeTimer(200 * time.Millisecond)
	defer timerStop()

	select {
	case <-timer.C:
		t.Fatal("timed out waiting for decryptWrappedKeyTask to send its error")
	case err := <-errorCh:
		must.ErrorContains(t, err, "context canceled")
	}
}


Review comment (refactor):
This looks fine, but check my reasoning: if we don't wait to close the context, do we end up getting the same "context canceled" error from `helper.Backoff`? If so, then waiting is about giving this test a fighting chance to be testing the thing we think it is, and not about deflaking the error output, right?

Code after:
	ctx, cancel := context.WithCancel(context.Background())
	cancel()

	// Ensure we receive an error indicating we hit the context done case and
	// check no cipher response was sent.
	err = encrypter.decryptWrappedKeyTask(ctx, kmsWrapper, key.Meta, wrappedKey, respCh)
	must.ErrorContains(t, err, "operation cancelled")
	must.Eq(t, 0, len(respCh))

	// Recreate the response channel so that it is no longer buffered. The
	// decrypt task should now block on attempting to send to it.
	respCh = make(chan *cipherSet)

	// Generate a new context and an error channel so we can gather the response
	// of decryptWrappedKeyTask running inside a goroutine.
	ctx, cancel = context.WithCancel(context.Background())

	errorCh := make(chan error, 1)

	// Launch the decryptWrappedKeyTask routine.
	go func() {
		err := encrypter.decryptWrappedKeyTask(ctx, kmsWrapper, key.Meta, wrappedKey, respCh)
		errorCh <- err
	}()

	// Roughly ensure the decrypt task is running for enough time to get past
	// the cipher generation. This is so that when we cancel the context, we
	// have passed the helper.Backoff functions, which are also designed to exit
	// and return if the context is canceled. As Tim correctly pointed out; this
	// "is about giving this test a fighting chance to be testing the thing we
	// think it is".
	//
	// Canceling the context should cause the routine to exit and send an error
	// which we can check to ensure we correctly unblock.
	timer, timerStop := helper.NewSafeTimer(500 * time.Millisecond)
	defer timerStop()

	<-timer.C
	cancel()

	timer, timerStop = helper.NewSafeTimer(200 * time.Millisecond)
	defer timerStop()

	select {
	case <-timer.C:
		t.Fatal("timed out waiting for decryptWrappedKeyTask to send its error")
	case err := <-errorCh:
		must.ErrorContains(t, err, "context canceled")
	}
}