df/add-stream-test-vectors

[sappenin]

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

[emschwartz]

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

[sappenin]

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]

@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]

@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]

@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]

@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]

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

[sappenin]

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

[sentientwaffle]

A few minor changes, but overall LGTM.

[sentientwaffle]

LGTM. I'm not sure why circleci believes so many files changed (causing it to fail).