search

interledger / rfcs

Specifications for Interledger and related protocols
https://interledger.org
Other
455 stars 111 forks source link

df/add-stream-test-vectors #555

Closed sappenin closed 5 years ago

sappenin commented 5 years ago

Introduces a JSON file that allows STREAM implementations to validate their ASN.1 OER encoding/decoding of alll Frames defined in IL-RFC-29.

Signed-off-by: sappenin sappenin@gmail.com

sappenin commented 5 years ago

It seems like this should test whole packets (with encryption) as well as the inner frames

Good call - I'll update that today and tag you when it's ready.

sentientwaffle commented 5 years ago

@sappenin consider using part or all of https://github.com/interledgerjs/ilp-protocol-stream/blob/master/test/fixtures/packets.json (generated by https://github.com/interledgerjs/ilp-protocol-stream/blob/master/scripts/generate-fixtures.ts). It might be excessive, but it does cover some important gotchas around e.g. exceeding max uint64s (https://github.com/interledgerjs/ilp-protocol-stream/blob/master/test/fixtures/packets.json#L810-L846).

sappenin commented 5 years ago

@sappenin consider using part or all of https://github.com/interledgerjs/ilp-protocol-stream/blob/master/test/fixtures/packets.json

@sentientwaffle Wow, that test harness is much better than the one I created.

I see there's a generator, so is it correct to say that we can just take the packets.json and use that as a fixed fixture, or is there some important reason to re-generate fixtures on every test run?

sentientwaffle commented 5 years ago

@sappenin Thanks! The generator is just for convenience -- to make it easier to add additional test cases. Adding the packets.json file is sufficient, there's no need to run (or even include) the generator script.

sappenin commented 5 years ago

@sentientwaffle Sounds good - one question about the frame:connection_new_address:empty test-case in here. Is that something that should be tolerated? This is the only thing that the Java code barfs on, but I'm thinking that a ConnectionNewAddress frame with no address should be a protocol violation, no?

sentientwaffle commented 5 years ago

You're correct, an empty address should be invalid. Not sure what I was thinking when I added that test case.

sappenin commented 5 years ago

@sentientwaffle I just pushed new changes to this PR that mirror your test vectors. Want to give this another look?