eventing icon indicating copy to clipboard operation
eventing copied to clipboard

Add the broker classes consideration into the test cases

Open Leo6Leo opened this issue 1 year ago • 2 comments

Problem Adding more test cases to test out the broker classes configuration in the default broker class configuration. The tests that get involved are unit tests.

Files involved: pkg/apis/eventing/v1/broker_defaults_test.go

Persona: Which persona is this feature for? Contributors

Exit Criteria A measurable (binary) test that would indicate that the problem has been resolved.

Time Estimate (optional): How many developer-days do you think this may take to resolve? 1

Additional context (optional) https://github.com/knative/eventing/pull/7631#discussion_r1481686685

/triage accepted

Leo6Leo avatar Feb 07 '24 16:02 Leo6Leo

I would like to work on this.@Leo6Leo can I know which broker classes are you talking about in broker_defaults_test.go so that I would get an idea about writing those test cases?

NeerajNagure avatar Apr 03 '24 05:04 NeerajNagure

@NeerajNagure Thanks for the help! I am providing some extra information here, and hopefully that can help you better understand the issue!

This issue is a expansion on this Pull Request: https://github.com/knative/eventing/pull/7631. I was trying to write the feature that users can set the customized broker configuration for different broker classes.

Let's take this as an example: We have cluster default broker class as MTChannelBasedBroker, and the configuration is for MTChannelBasedBroker.

Before this PR, if you hope to create a Kafka Broker without providing the configuration, knative will use the cluster default broker class configuration, which is MTChannelBasedBroker's configuration.

After this PR, you will have the chance to set multiple broker configuration template as default for different broker classes. Right now, not only we have cluster default broker class as MTChannelBasedBroker, and the configuration is for MTChannelBasedBroker, we have a new field called BrokerClasses, and this contains a value of Kafka Broker and it's config. So instead of using MTChannelBasedBroker's config, knative now knows to use Kafka Broker's config.

https://github.com/knative/eventing/blob/4a2da5f67fb9bba297c42abf3ac0b96f0a791536/pkg/apis/eventing/v1/broker_defaults_test.go#L283

As you can see in this file above, it contains a lot of unit tests, but the tests for this new feature is missing. So we would need your help!

Learn more about that PR in this doc., and let me know if you have any questions.

Leo6Leo avatar Apr 12 '24 18:04 Leo6Leo