Latest commit breaks servers.IDFromName

[pedantic79]

The latest commit #156 seems to have broken a test case I was using. The testcase uses: servers.IDFromName.

It might be a regression caused by the PR or maybe the test case needs to be updated. Either way, any help would be appreciated.

A minimal sample is available here: https://github.com/pedantic79/gophercloud-utils-test

 ~/gophercloud-utils-test ‹main› » go test
--- FAIL: TestGetServerIDByName (0.00s)
    sample_test.go:46: Expected 9e5476bd-a4ec-4653-93d6-72c93aa682ba, but was
    convenience.go:35: Failure in sample_test.go, line 49: unexpected error "Found 3 servers matching derp"
--- FAIL: TestGetServerIDsByName (0.00s)
    sample_test.go:61: Expected 1, but was 3
FAIL
exit status 1
FAIL	github.com/pedantic79/gophercloud-utils-test	0.233s

The branch "old" contains the same test has a dependency on the previous commit (so it doesn't include the second test).

~/gophercloud-utils-test ‹old› » go test
PASS
ok  	github.com/pedantic79/gophercloud-utils-test	0.173s

[jtopjian]

Thanks for the heads-up.

The good news is that it's just the unit test that is broken. #156 introduced name filtering by way of ListOpts. The unit test doesn't recognize the filtering and returns all servers in the fixture.

Probably the best way to resolve would be to add a custom fixture and unit test that emulated the server-side filtering being done by the ListOpt. IMO, this kind of test is kind of moot since the result is being tailored for the IDFromName function... but the test could have comments describing the behaviour it is emulating.

Thoughts?

[pedantic79]

Okay. I see why the old code works. The old IDFromName would go through all the responses, and then count the matching responses. While the new one assumes ListOpts actually returns the correct values. Thanks.

I'm going to close this.