Repository: grpc/grpc-go
File: internal/transport/transport_test.go (Go)

Code before:
			var serverStream *ServerStream
			waitWhileTrue(t, func() (bool, error) {
				serverTransport.mu.Lock()
				defer serverTransport.mu.Unlock()
				for _, v := range serverTransport.activeStreams {
					if v.id == clientStream.id {
						serverStream = v
						return false, nil
					}
				}
				return true, nil
			})

			if serverStream == nil {
				t.Fatalf("Server stream not found for client stream ID %d", clientStream.id)
			}

			// For the NonActiveStream test, remove the stream from activeStreams
			if test.removeFromActive {
				serverTransport.mu.Lock()
				delete(serverTransport.activeStreams, serverStream.id)
				serverTransport.mu.Unlock()
			}

			initialSucceeded := serverTransport.channelz.SocketMetrics.StreamsSucceeded.Load()
			initialFailed := serverTransport.channelz.SocketMetrics.StreamsFailed.Load()

			serverTransport.deleteStream(serverStream, test.eosReceived)
			serverTransport.deleteStream(serverStream, test.eosReceived)
			serverTransport.deleteStream(serverStream, test.eosReceived)

			finalSucceeded := serverTransport.channelz.SocketMetrics.StreamsSucceeded.Load()
			finalFailed := serverTransport.channelz.SocketMetrics.StreamsFailed.Load()

			gotSucceededDelta := finalSucceeded - initialSucceeded
			gotFailedDelta := finalFailed - initialFailed

			if gotSucceededDelta != test.wantStreamSucceeded {
				t.Errorf("StreamsSucceeded: got delta %d, want %d", gotSucceededDelta, test.wantStreamSucceeded)
			}
			if gotFailedDelta != test.wantStreamFailed {
				t.Errorf("StreamsFailed: got delta %d, want %d", gotFailedDelta, test.wantStreamFailed)
			}

			// Verify stream was removed from activeStreams (unless it was pre-removed for NonActiveStream test)
			if !test.removeFromActive {
				serverTransport.mu.Lock()
				_, exists := serverTransport.activeStreams[serverStream.id]
				serverTransport.mu.Unlock()
				if exists {
					t.Error("Stream should have been removed from activeStreams")

Review comment (suggestion):
These values will always be 0, right? If so, we can assert they are 0 and assert that the absolute values of the metrics later instead of deltas. This would make it clearer for readers to see exactly what is the expected value of the metric.

Code after:
					}
				}
				return true, nil
			})

			if serverStream == nil {
				t.Fatalf("Server stream not found for client stream ID %d", clientStream.id)
			}

			// First call to deleteStream should remove the stream from activeStreams and update metrics
			serverTransport.deleteStream(serverStream, test.eosReceived)

			// Check metrics after first deleteStream call
			streamsSucceeded := serverTransport.channelz.SocketMetrics.StreamsSucceeded.Load()
			streamsFailed := serverTransport.channelz.SocketMetrics.StreamsFailed.Load()

			if streamsSucceeded != test.wantStreamSucceeded {
				t.Errorf("After first deleteStream - StreamsSucceeded: got %d, want %d", streamsSucceeded, test.wantStreamSucceeded)
			}
			if streamsFailed != test.wantStreamFailed {
				t.Errorf("After first deleteStream - StreamsFailed: got %d, want %d", streamsFailed, test.wantStreamFailed)
			}

			// Additional calls to deleteStream should not change metrics (stream already deleted)
			serverTransport.deleteStream(serverStream, test.eosReceived)
			serverTransport.deleteStream(serverStream, test.eosReceived)

			// Verify metrics haven't changed after subsequent calls
			additionalStreamsSucceeded := serverTransport.channelz.SocketMetrics.StreamsSucceeded.Load()
			additionalStreamsFailed := serverTransport.channelz.SocketMetrics.StreamsFailed.Load()

			if additionalStreamsSucceeded != test.wantStreamSucceeded {
				t.Errorf("After multiple deleteStream calls - StreamsSucceeded changed: got %d, want %d", additionalStreamsSucceeded, test.wantStreamSucceeded)
			}
			if additionalStreamsFailed != test.wantStreamFailed {
				t.Errorf("After multiple deleteStream calls - StreamsFailed changed: got %d, want %d", additionalStreamsFailed, test.wantStreamFailed)
			}
		})
	}
}