test: add unit test for GenerateScaledObjectName

[converge]

Fixes #

Proposed Changes

• add unit test for GenerateScaledObjectName

note: this is a PR to validate how unit tests should be written for knative. Upcoming unit test is for Keda integration.

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: converge
Once this PR has been reviewed and has the lgtm label, please assign lionelvillard for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov]

Codecov Report

Attention: Patch coverage is 4.91803% with 58 lines in your changes are missing coverage. Please review.

Project coverage is 49.37%. Comparing base (02a879b) to head (8543c2c).
Report is 116 commits behind head on main.

Files	Patch %	Lines
...ntrol-plane/pkg/autoscaler/keda/mocks/keda_mock.go	3.33%	58 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #3892       +/-   ##
=============================================
- Coverage     61.99%   49.37%   -12.63%     
=============================================
  Files           188      247       +59     
  Lines         12634    14909     +2275     
  Branches        273        0      -273     
=============================================
- Hits           7833     7361      -472     
- Misses         4187     6792     +2605     
- Partials        614      756      +142     

Flag	Coverage Δ
java-unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[knative-prow]

@converge: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
upgrade-tests_eventing-kafka-broker_main	8543c2c	link	true	/test upgrade-tests

Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[Cali0707]

This looks good @converge - thanks!

I'm not very familiar with when we add mocks to the tests in this repo, so I'm going to let @pierDipi take a look at the test mocks before we approve this

/assign @pierDipi

[Cali0707]

Nice use of the table testing pattern! This is how we normally do unit tests in eventing (as I'm sure you noticed)

[pierDipi]

Did you consider having a test in https://github.com/knative-extensions/eventing-kafka-broker/blob/main/control-plane/pkg/reconciler/consumergroup/consumergroup_test.go that would assert that the ConsumerGroup reconciler creates all KEDA objects when the feature flag is enabeled?

Such test would cover also the GenerateScaledObjectName since we need to explicitly indicate the object names (and spec), similar to how is done here for Consumers

eventing-kafka-broker/control-plane/pkg/reconciler/consumergroup/consumergroup_test.go

Lines 112 to 137 in 865fea6

	WantCreates: []runtime.Object{
	NewConsumer(1,
	ConsumerFinalizer(),
	ConsumerSpec(NewConsumerSpec(
	ConsumerTopics("t1", "t2"),
	ConsumerConfigs(
	ConsumerBootstrapServersConfig(ChannelBootstrapServers),
	ConsumerGroupIdConfig("my.group.id"),
	),
	ConsumerVReplicas(1),
	ConsumerPlacement(kafkainternals.PodBind{PodName: "p1", PodNamespace: systemNamespace}),
	)),
	),
	NewConsumer(2,
	ConsumerFinalizer(),
	ConsumerSpec(NewConsumerSpec(
	ConsumerTopics("t1", "t2"),
	ConsumerConfigs(
	ConsumerBootstrapServersConfig(ChannelBootstrapServers),
	ConsumerGroupIdConfig("my.group.id"),
	),
	ConsumerVReplicas(1),
	ConsumerPlacement(kafkainternals.PodBind{PodName: "p2", PodNamespace: systemNamespace}),
	)),
	),
	},

The benefit is that we don't need to have mocks, while having the benefit of a "unit test" that is fast and doesn't interact with the real k8s API