test(inputs.redfish): Prepare redfish refactoring - #19491
Conversation
This test would fail on gofish since the lib only starts using auth on later endpoints.
inhinias
changed the title
inhinias
changed the title
inhinias
changed the title
telegraf-tiger
Bot
added
fix
srebhan
changed the title
srebhan
added
the
ready for final review
| mu.Lock() | ||
| defer mu.Unlock() | ||
| require.Equal(t, []string{"/redfish/v1/Systems/1"}, requested) | ||
| require.NotContains(t, requested, "/redfish/v1/Chassis") |
There was a problem hiding this comment.
NotContains on a []string compares whole elements, not substrings, so this can only fail if a request path is exactly /redfish/v1/Chassis, which we never produce. I ran it with /redfish/v1/Chassis/1/ and /redfish/v1/Chassis/1/Thermal in the slice and it still passed, so the test no longer catches the thing it is named after. Sven's NotContains suggestion is still the right one, it just needs to run per element:
for _, path := range requested {
require.NotContains(t, path, "/redfish/v1/Chassis")
}Worth noting the line above is stale too, the new fixture has no Links at all rather than an empty reference, so nothing resolves to the web root any more.
There was a problem hiding this comment.
Despite the new for loop being stale I would still implement it in the way you suggested. Because if gofish requests the legacy thermal/power api while only the subsystem api is available, it returns nil on that request. This case should be caught and produce a warning until the newer endpoints are implemented.
I will rename the testcase for that specific situation in a later PR as it is no longer about landing at the root via an empty link/ref.
| require.NoError(t, err) | ||
| err = r.Gather(&acc) | ||
| require.EqualError(t, err, "received status code 401 (Unauthorized) for address http://"+u.Host+"/redfish/v1/Systems/System.Embedded.1, expected 200") | ||
| require.ErrorContains(t, err, "401") |
There was a problem hiding this comment.
[nit] Matching on bare "401" can hit the httptest port instead of the status code, since those land in the 49152+ range and something like 54012 contains it. require.ErrorContains(t, err, "received status code 401") keeps your point about not pinning the whole message while staying unambiguous.
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
3 participants
Summary
In preparation of the upcoming refactor there are some more test cases that require adaption.
This will also be relevant for TestSkipChassisWithoutThermalAndPowerReference but requires a bigger change of the test. I'll spin up a quick separate PR when i get around to it.
Checklist