Sort map keys during testing so output is deterministic

kolo / xmlrpc

Implementation of XMLRPC protocol in Go language.

MIT License

158 stars 92 forks source link

Sort map keys during testing so output is deterministic #59

Closed icholy closed 5 years ago

icholy commented 5 years ago

Since map iteration order is randomised, the tests randomly pass/fail. This change sorts the map keys before encoding to keep the output deterministic. Now the test always pass.

kolo commented 5 years ago

Thank you for contribution. I don't like an idea of adding unnecessary operation to the encoding just to make the tests passing. I think this should be solved on the test level.

icholy commented 5 years ago

@kolo are you opposed because of the performance overhead?

kolo commented 5 years ago

@icholy mostly, but it also doesn't look good from my point of view.

icholy commented 5 years ago

My original plan was to have a unexported flag to only enable this behavior during tests and I decided against that for the following reasons:

Having a consistent output seems cleaner from a user's perspective.
I think that the performance overhead is negligible (in the common case).
Go 1.12 changed the fmt.Printf function to sort map keys for the exact same reason.

I don't have a strong preference either way. I'll add the flag if you'd prefer that approach.

kolo commented 5 years ago

I think having a flag is a good compromise.

icholy commented 5 years ago

@kolo since these are only running during tests, I've switched to using sort.Slice to shorten up the code.