Add a way to validate XContent rendering produces valid JSON/YAML

[dakrone]

It's common in tests to have a method that tests serialization and deserialization of the actual Java objects, however, it's quite possible to make a mistake in the toXContent method that does not produce either valid JSON or YAML. Right now the only real way to do this is with a REST test (other than a lot of string comparison in a unit test), which is a lot of overhead for something like this.

It would be great if we could have some kind of AssertingToXContent wrapper that could test this to ensure we don't ever generate JSON like:

{ : { "bar": "foo"} }

[javanna]

As part of the search refactoring, we introduced quite some tests around query / search request serialization. As part of those we generate a random builder, call toXContent against it, parse it, then check that the generated query is equivalent to the initial one. I think this achieves the goal without string comparisons as we compare only the java objects through the equals method (that are also separately tested).

This is not possible though in all cases as some objects implement ToXContent although we have no parsing code for them (e.g. response objects that only get printed out). IN that case we should simply call toXContent and check its output? Or call it before and after serialization and check that it's valid json and equivalent on both ends?

I am interested in the AssertingToXContent idea. How would that work exactly? Wouldn't we need to write specific tests for each class that implements ToXContent? Can you elaborate @dakrone ?

[javanna]

I am digging the AssertingToXContent idea, but I think it is tricky unless the ToXContent contract is better defined. See #16347. In many cases the output of toXContent is not a valid object. Wrapping it into a new anonymous object would make it valid, unless it starts already its own anonymous object. In other words I don't think this is feasible at the moment. Thoughts @dakrone ?

[elasticmachine]

Pinging @elastic/es-core-infra

[jaymode]

Given the lack of further discussion and ideas for this, I am going to close this issue as I don’t believe the core/infra team will prioritize and have bandwidth to work on this anytime soon.