Implement listbanned method and test

[GideonBature]

The JSON-RPC method listbanned does return a type. We want to test this to catch any changes in behavior in future Core versions.

This PR adds a client function that errors if the return value is anything other than the type it returns, along with an integration test that calls this function.

Ref: #116

[tcharding]

There are a few problems with this one. Primarily the integration test does not test anything because the vector in ListBanned is always empty so the Banned struct is never tested.

Can you fix that up and we'll go from there?

Also:

• the client macro needs underscore in the name
• the last patch should be part of the first

Thanks

[GideonBature]

oh my... Let me fix them up.

[tcharding]

Looking much better man.

[tcharding]

I think something is still wrong here. If I comment out the fields in Banned and run the tests it still passes?

[GideonBature]

Woaw... Let check what is wrong.

[GideonBature]

I think something is still wrong here. If I comment out the fields in Banned and run the tests it still passes?

Thank you for pointing this out. seems the test was only checking if there is at least an element inside of the ListBanned, which would be the Banned entry, but not verifying whether the Banned struct itself contains valid field data. I added an assertion for the address field since it is present across all versions. Still thinking of a more robust way to handle this.

[tcharding]

The fact that the list is not empty should imply that at least one Banned struct was deresrialized. That is why I thought it strange that removing fields didn't make the test fail. I'm not 100% sure I'm not misunderstanding something though. Perhaps leave it with me and I'll investigate.

[GideonBature]

True, I was surprised with the behaviour too. No problem. Thank you...

[tcharding]

Needs rebase. Can you post when you force push with what changed please. I'm still confused by this PR.

[GideonBature]

Before now what I did was to improve the test to check for the address field if it correctly returns the address that is being banned.

Let me rebase now...

[jamillambert]

This looks good now, just a couple of minor suggestions.

The test works now as far as I can tell, but would be improved with the previously suggested change.

[GideonBature]

This looks good now, just a couple of minor suggestions.

The test works now as far as I can tell, but would be improved with the previously suggested change.

Thank you for your review, suggestions have been effected.

[tcharding]

In v17 listbanned returns null so I'd expect to see that verified in the client macro. Then in the v17 types docs I expected to see 'returns nothing'. Then in what ever version introduces the result I'd expect to see an additional client macro for that version and have it used for subsequent versions. Then the test would probably have to handle the difference macro calls also.

[jamillambert]

I think something is still wrong here. If I comment out the fields in Banned and run the tests it still passes?

This seems to be a bigger issue with the tests in general and not just localized to this PR. See issue #241

[GideonBature]

In v17 listbanned returns null so I'd expect to see that verified in the client macro. Then in the v17 types docs I expected to see 'returns nothing'. Then in what ever version introduces the result I'd expect to see an additional client macro for that version and have it used for subsequent versions. Then the test would probably have to handle the difference macro calls also.

Alright, in the docs, for v17 not much was said about the listbanned including what it is returning, so I decided to check the core, and found out listbanned is returning a json-array. Then for subsequent versions there are some changes made to the fields of this json.

[jamillambert]

In v17 listbanned returns null so I'd expect to see that verified in the client macro. Then in the v17 types docs I expected to see 'returns nothing'. Then in what ever version introduces the result I'd expect to see an additional client macro for that version and have it used for subsequent versions. Then the test would probably have to handle the difference macro calls also.

Alright, in the docs, for v17 not much was said about the listbanned including what it is returning, so I decided to check the core, and found out listbanned is returning a json-array. Then for subsequent versions there are some changes made to the fields of this json.

I can confirm that v17 does return all of the fields in Banned and it is just the core v17 help that is incomplete/unhelpful.

[tcharding]

Sorry mate, needs rebase again. I'll try get this one in next.

[jamillambert]

ACK f76918a