Test using Core's test cases

[tcharding]

Currently we have only limited test coverage. And for the RPC methods that are tested we only test a subset of things e.g.,

• We don't test all optional fields
• We don't ensure all fields are non-default e.g., they may be being filled in by serde and not actually deserialized.

An idea, courtesy of @stevenroose, is to grab the tests from Core used for testing the RPC code and then try to parse all of those.

[GideonBature]

The idea is great, as it will help us ensure that:

• most if not all optional fields are tested.
• prevents issues where fields might not be correctly deserialised but still seem to work due to default values.
• we will be certain that it follows correctly Bitcoin Core's RPC Specification.

We can Start the implementation, I will be like to take on the task if it's okay.

[tcharding]

Go wild.

[jamillambert]

I think this issue is now dead? We have a pretty thorough test suite now and I don't want to go down this route personally.

[tcharding]

This is still an idea I'd like to pursue. What it means is we currently typically have a single test, this issue describes getting test vectors from Core i.e., having a whole bunch of tests for each method instead of 1.

[jamillambert]

I'm still not convinced. I think the effort should go into other improvements in the tests that check if the structs in this crate are correct, e.g. #137, #291 and #365.

Using test vectors from Core might be a way to do this in some RPCs, but I just think that the main reason to test all RPCs with a range of inputs using the test vectors is to test the functionality of core.

For this crate we really only have 3 cases we care about, the returned field is empty, valid, or there's an error.

[tcharding]

I just think that the main reason to test all RPCs with a range of inputs using the test vectors is to test the functionality of core.

You are correct. However there is a reason I think its worth doing .Its a quick and easy (hopefully) way to get a bunch of different things out of Core without having to make up RPC calls ourselves. In particular, to get good coverage for a method requires understanding exactly when and why Core returns what ever it does. And this requires a bunch of thought, where as just using all the test vectors we can do it mechanically with no thought into why the exact data is returned.

[jamillambert]

I agree that all makes sense and you've now convinced me, assuming the test vectors do have cases for both returning and not returning all optional fields.

[tcharding]

Yeah, I haven't looked at them yet.