Add Cluster Allocation Explain API Tests

[joshua-adams-1]

Adds tests for the cluster/allocation/explain API for when non-integer values are passed as the integer-expected shard parameter. This change does not modify the cluster/allocation/explain API itself, but adds tests for current, expected behaviour. This PR is a pre-requisite for a change modifying the behaviour of the API when a float is passed, resolving a comment on #129342

[DaveCTurner]

IMO this is a rather excessive way to check a single line of code which calls PARSER.declareInt. If you think the tests for AbstractObjectParser do not adequately cover these corner cases then we should address that there, but I don't think it makes sense to test all the failure paths and other corners through the API like this. There are 351 other production calls to that one method that we'd have to test similarly after setting this precedent, and yet I'm struggling to imagine how any of these tests might fail.

[elasticsearchmachine]

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

[joshua-adams-1]

There are 2 reasons for adding these tests:

1. We currently allow the user to enter a float value for a field we've marked as integer. There was a comment on #129342 to change this, and if I did that without having tests like this in place, nothing would break and we wouldn't be notified.
2. Just because there are tests in place for the Parser, doesn't mean we shouldn't have tests in place for the class using the parser. Changes made by someone in the ClusterAllocationExplainRequest class are currently untested. I also think it's important to explicitly state what we expect the API to do in situations where non-integer types are passed in fields expecting integers, rather than it being assumed because it's mentioned in the documentation

[DaveCTurner]

I see ok, yeah just seems a bit odd to nail down the current behaviour even though we don't really care about the behaviour in these corner cases today.

Just because there are tests in place for the Parser, doesn't mean we shouldn't have tests in place for the class using the parser.

True, but it doesn't mean that we should have such tests either :) If you extrapolate that line of thinking you end up having to test everything at every level through every other layer of abstraction which is clearly infeasible. So there's some judgement required about whether such tests are worth it. Often they are, under-testing is more common than over-testing, but both extremes are problematic in their own ways.

[DaveCTurner]

LGTM

[joshua-adams-1]

I understand what you're saying about over-testing, and if we were to extrapolate this to all APIs, then I appreciate this would be a massive bloat to our testing suite! My logic this time was 1) I am, by nature, a big advocate of tests and will always ere on the side of more tests, a line of thinking I'm happy to re-evaluate as I learn more about the codebase and 2) we're directly changing the functionality tested by these tests so in this instance it seemed a necessary addition so that I can verify my new code works as expected. Happy to discuss anything further offline! Thank you for the review