Skip to content

[chore] Add a new Test and refactor code function #25101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 0 commits into from

Conversation

ZenoCC-Peng
Copy link
Contributor

Description:
Add a new test 'testInvalidMethod' in receiver_test.go, and refactor code for readability.

Link to tracking Issue: 20760

Testing: All test pass.

@@ -9,7 +9,7 @@ change_type: enhancement
component: receiver/collectdreceiver
Copy link
Contributor

Choose a reason for hiding this comment

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

the format of changlog seems not right? Do you generate this file by make chlog-new?

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, I think this pr do not need changelog, since there is no changes in terms of the users

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it looks like Zeno updated his existing changelog entry. Every PR needs to have its own changelog or no changelog as @fatsheep9146 explains. I agree this PR doesn't need a changelog.

wantData: []pmetric.Metrics{},
}

t.Run(testInvlidMethodCase.name, func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
t.Run(testInvlidMethodCase.name, func(t *testing.T) {
t.Run(testInvalidMethodCase.name, func(t *testing.T) {


func testInvalidMethod(t *testing.T, sink *consumertest.MetricsSink) {

testInvlidMethodCase := testCase{
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
testInvlidMethodCase := testCase{
testInvalidMethodCase := testCase{

testInvalidMethod(t, sink)
}

func testInvalidMethod(t *testing.T, sink *consumertest.MetricsSink) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can add this to the test cases above instead

t.Run(testInvlidMethodCase.name, func(t *testing.T) {
sink.Reset()
req, err := http.NewRequest(
"GET",
Copy link
Member

Choose a reason for hiding this comment

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

Can we parameterize this method as another testCase field instead of adding another test case?

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

Successfully merging this pull request may close these issues.

4 participants