Skip to content

test(inputs.redfish): Extend coverage in preparation of lib change#19163

Open
inhinias wants to merge 7 commits into
influxdata:masterfrom
inhinias:rf_gf_tests_adaption
Open

test(inputs.redfish): Extend coverage in preparation of lib change#19163
inhinias wants to merge 7 commits into
influxdata:masterfrom
inhinias:rf_gf_tests_adaption

Conversation

@inhinias

Copy link
Copy Markdown
Contributor

Summary

Since the plan is to move to the gofish lib we extend the tests to work with them in a later pr.
Because gofish expects to move from the top of the api to lower nodes, every testcase requires
/redfish/v1/ & /redfish/v1/Systems/ to be present.

Also gofish does no trigger an invalid Response with a trailing "{". Therefore its moved to the start of the data.

Datacenter is supposed to be under the Postal address: tags["datacenter"] = chassis.Location.PostalAddress.DataCenter but its nowhere to be found in the standard. See page 35.

It should be considered if the "room" tag should be changed too since PostalAddress is deprecated.
This pr removes just the datacenter tag from the tests but not the actual program. That test would fail and is therefor skipped. Please inform me on how to handle this case for the "moving to gofish" pr.

Also i already added the testdata for the subsystems that will be added as a feature later.

Checklist

Related issues

Related to: #18963

@telegraf-tiger telegraf-tiger Bot added fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Jun 26, 2026
@inhinias inhinias closed this Jun 26, 2026
@inhinias inhinias reopened this Jun 26, 2026
@inhinias inhinias marked this pull request as ready for review June 26, 2026 12:53

@srebhan srebhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @inhinias! Please try to not change existing tests! Find my comments in the code...

Comment thread plugins/inputs/redfish/testdata/dell/dell_thermalinvalid.json Outdated
Comment thread plugins/inputs/redfish/testdata/hp/hp_powerinvalid.json
Comment thread plugins/inputs/redfish/testdata/hp/hp_systemsinvalid.json
Comment thread plugins/inputs/redfish/testdata/hp/hp_thermalinvalid.json
Comment thread plugins/inputs/redfish/redfish_test.go Outdated
Comment on lines +919 to +951
ComputerSystemID: "System.Embedded.2",
ComputerSystemID: "1",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we please keep the previous version?

Comment thread plugins/inputs/redfish/redfish_test.go Outdated
}

func TestIncludeTagSetsConfiguration(t *testing.T) {
t.Skip("Datacenter tag removed, not part of redfish. Fix will arrive later")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm... The test should not fail, at least not if you don't change the plugin code, should it? If this is an issue arising when changing the underlying lib, please add the "skip" with that PR to document it is breaking things...

"rack": "",
"room": "",
"row": "",
"datacenter": "",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please don't change existing tests! Those are there for making sure your changes are backward compatible and thus should not be changed. If you need to change an existing test it means your plugin modifications are breaking!

Same for the other occurrences below.

@srebhan srebhan changed the title fix(inputs.redfish): Extend testcases for gofish lib change test(inputs.redfish): Extend coverage in preparation of lib change Jul 1, 2026
@srebhan srebhan self-assigned this Jul 1, 2026
@telegraf-tiger telegraf-tiger Bot added the test label Jul 1, 2026
@srebhan srebhan removed the fix pr to fix corresponding bug label Jul 1, 2026
@srebhan

srebhan commented Jul 2, 2026

Copy link
Copy Markdown
Member

@inhinias I marked the resolved items as "resolved". However, there are three issues left...

@inhinias

inhinias commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@srebhan SystemID, Datacenter Tags and test skip reverted.

@telegraf-tiger

telegraf-tiger Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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

Labels

plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants