use pinpointed handling of non-serializable data

[RonnyPfannschmidt]

fixes #12

the caveat is that now we sometimes have magic json objects in the depth of a message to fix the details

this also exposed a bug in warning record message handling that was hidden by the broad str magic

[nicoddemus]

@RonnyPfannschmidt can you please rebase on main? Thanks!

[RonnyPfannschmidt]

@nicoddemus done

[nicoddemus]

LGTM, let's see what @The-Compiler has to say.

[The-Compiler]

Sorry for the delay, just came back from Berlin for yet another pytest company training on Saturday :sweat_smile:

Just dug into the git history of the project where this happened to me, and I basically accidentally serialized a set instead of a list. I suppose this behavior makes sense. I wonder if a TypeError might make even more sense, but I suppose it would be a bit in the way if people use record_property for other stuff?

Either way, this seems fine to me. Perhaps "handle unserializable objects with a pinpointed marker" in the changelog should be a bit more specific so people know what to watch out for?

[RonnyPfannschmidt]

Perhaps a little bikeshedding can help

It might also be a good idea to switch from str to safe_repr

[The-Compiler]

Some questions to ponder on:

• Why are we doing this instead of raising a TypeError?

Probably so someone with previous usage of record_property can start using the plugin without things exploding?

• Do we foresee people using the actual values, or is this basically just included as debugging output?

I'd say debugging output?

• If the latter, would it suffice if we just included the keys instead, and leave the values out entirely? After all, they might be rather noisy, if e.g. containing Python memory addresses which change every time.

IMHO it would be fine? But I can see there is value in seeing the value (hah) too.

[RonnyPfannschmidt]

If we switch from str to length limited safe repr, I think we will be fine

[nicoddemus]

Probably so someone with previous usage of record_property can start using the plugin without things exploding?

It was introduced here:

https://github.com/pytest-dev/pytest-reportlog/commit/bac07c31f0d70d31facf8ce3fa5ac913c4a411d3

~~I don't recall the details, but probably xdist was including JSON-incompatible data into the reports.~~ Problem is described here: #5

[RonnyPfannschmidt]

@nicoddemus xdist adds report.node but doesnt expand report serialization to turn it into json compatible objects atm

[nicoddemus]

Yes (you probably did not see my edit where I found the reason after the initial post hehehe)