More resilient stdout parsing of dialer stdout#224
Conversation
MarcoPolo
force-pushed
the
marco/json-unexpected-end
branch
from
7f1842d to
e7865dc
Compare
I still think deno could be a good fit for the problem we have here but I agree with your particular solution :) |
|
Could I get a review from someone? friendly ping to @thomaseizinger or @mxinden |
|
Merging this for now since it's been causing a lot of false negatives. I'm still interested in a review please :) |
thomaseizinger
left a comment
thomaseizinger
left a comment
There was a problem hiding this comment.
Sorry for the late review, one suggestion but thank you for the fix!
| "main": "testplans.ts", | ||
| "scripts": { | ||
| "test": "ts-node testplans.ts", | ||
| "test": "ts-node src/compose-stdout-helper.ts && ts-node testplans.ts", |
There was a problem hiding this comment.
Does shell glob expansion work in here? We could move these files to a tests dir and just run all of them.
There was a problem hiding this comment.
For that, I'd then probably be cleaner to keep the helper in src and just make another small file that runs the function.
There was a problem hiding this comment.
That makes sense. Will do this change once we have more than one test file :)
* More resilient stdout parsing of dialer stdout * Add another test case * Handle JS cases where timings aren't the only thing on stdout
3 participants
First off, I don't really understand why compose split the line here. It may be a bug in zig-libp2p, but there are no other callers of stdout (no race). And there's nothing fancy going on, it just writes to the stdout file the bytes directly (no buffering).
Whatever the reason, we should probably be a bit more resilient with parsing of the output compared to the initial regex. This adds a helper to help with parsing compose's output as well as a test case to make sure that we don't silently break this.
To avoid adding a whole new dependency on some test framework that opens the door to the nightmare of JS bundling, the test is a simple function that is run if you run the file directly (as opposed to importing it). And it's run as part of
npm testwhen you run the interop tests (so it'll happen in CI as well).