Fix: set DucktapeJSONEncoder as default encoder in reports.

[ivandasch]

Fix #250

[ewencp]

Did one of the cases not using DucktapeJSONEncoder cause a problem?

imo we should really only go one of two ways here. The current approach is that cases where we expect complex data structures that the json module won't handle them naturally provide the override. This is used for cases where, e.g., we know we have a TestResult which is a "real" class rather than just a dumb data container. The danger is that we might miss a case, but to date, I haven't heard of any issues with this.

The second would be to be very defensive and support the to_json extension everywhere. If we wanted to do that, we should make it fully comprehensive -- in json_serializable.py we should provide the type of wrapper you have here and use it universally, probably banning direct imports of json everywhere else. That guarantees we cover not only the reporter, but also, e.g., result.py, vagrant.py, etc.

[ivandasch]

@ewencp Hi, Ewen

Did one of the cases not using DucktapeJSONEncoder cause a problem?

Currently, if you try to use object in @parametrize, test will fail. Namely, we use subclass of LooseVersion and we always should convert it to str in our project.
Also, we created custom pararemetrization and pass it from --globals (run all suite against specific version or against version vs version in compatibility suite).

I suppose that it is possible to remove this restriction and add ability to user pass object with to_json overriden method to parameter.

The second would be to be very defensive and support the to_json extension everywhere.

So you suggests to move my helper function to distinct module and use it everywhere? Ok, sounds good for me.

[ivandasch]

@ewencp Hi, I changed my PR as you suggested (second variant). Could you please look at it again?

[stan-is-hate]

pardon my trigger finger :)

[stan-is-hate]

I'm not super familiar with all of ducktape yet, can you please clarify how using DucktapeJSONEncoder helps with this:

Currently, if you try to use object in @parametrize, test will fail. Namely, we use subclass of LooseVersion and we always should convert it to str in our project.

?

Overall I don't have a strong opinion yet, but don't see much harm in using our own load(s) and dump(s) methods.

[cla-assistant]

All committers have signed the CLA.