Skip to content

Added unit tests for kafkaServer implementation#357

Open
Yashwanth-Ranjan-Singaravel wants to merge 12 commits intoopen-lambda:mainfrom
Yashwanth-Ranjan-Singaravel:feature/kafka-unit-tests
Open

Added unit tests for kafkaServer implementation#357
Yashwanth-Ranjan-Singaravel wants to merge 12 commits intoopen-lambda:mainfrom
Yashwanth-Ranjan-Singaravel:feature/kafka-unit-tests

Conversation

@Yashwanth-Ranjan-Singaravel
Copy link
Contributor

No description provided.

@Yashwanth-Ranjan-Singaravel Yashwanth-Ranjan-Singaravel marked this pull request as ready for review February 19, 2026 13:20
Copy link
Member

@tylerharter tylerharter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are too many tests to review at once. Can we start with 1 or 2? Simple stuff, like there is one message, and make sure there is one invocation as a results.

Also, can we update the GitHub CI so the go unit tests will run?

// NewKafkaManager creates and configures a new Kafka manager
func NewKafkaManager(lambdaManager *lambda.LambdaMgr) (*KafkaManager, error) {
var invoker LambdaInvoker
if lambdaManager != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have a nil check? Is it reasonable to get nil? We could just not check (like before), or just panic if nil.

"github.com/twmb/franz-go/pkg/kgo"
)

// syncBuffer is a thread-safe bytes.Buffer for capturing logs from goroutines.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all very complicated for some tests, can we get some simpler tests in first, and discuss whether we need more before doing this?

Copy link
Member

@tylerharter tylerharter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

}

func TestConsumeLoop_ContinuesThroughErrors(t *testing.T) {
var logBuf bytes.Buffer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have error count, or some other way to test that doesn't involve redirecting and capturing logs


// runConsumeLoop starts the consume loop in a goroutine and returns a stop
// function that signals shutdown and waits for the goroutine to exit.
func runConsumeLoop(consumer *LambdaKafkaConsumer) (stop func()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more comments to walk through the logic here

// --- Tests ---

func TestConsumeLoop_ProcessesRecords(t *testing.T) {
invoker := &MockLambdaInvoker{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better comments. we're testing the consumer. above layer (client) is mocked, and below layer (invoker) is mocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants