The reason we're capturing the Stdout is because that is how the LSP communicates with VScode. At the moment it is hardcoded into crates\p4-analyzer\src\stdio.rs, which is why the current commented out implementation in Tester uses a Cargo called gag. This crudely redirects the Stdout which enables us to test the LSP output. There are two blaring issues with the current implementation (which is why it's commented out):
The gag cargo only works when Cargo test uses --nocapture and running in serial (--test-threads=1)
To tell the Tester that a message has been sent, a first attempt event listener is built into stdio.rs. This was done incorrectly as it required p4-analyzer to depend on the tester!
Proposal
I can think of three solutions to the issue: Refactor stdio.rs so that Stdout isn't hardcode, properly implement an event listener to remove tester dependency, use Command::new("cargo").arg("run").
Refactor stdio.rs to change trait Write source
This solution would remove the need for capturing the Stdout and instead redirect the message writing source in stdio.rs directly. Is a sensible solution to the issue but do we want to rewrite production code just for testing purposes? Would be a very specific change that might only benefit stdio.rs unit tests and cause head-aches else where!
Properly implement an event listener
This solution is already tested to have worked however with the issues stated above. Doing an event listener properly would remove the dependency of tester in p4-analyzer cargo but still require the use of gag cargo.
Use Command::new("cargo").arg("run")
Would solve both of the issues but create a new one in that debugging code would be harder. The code would act as a black box only interacting through inputs and outputs!
Note
None of these solutions are perfect and welcome to any ideas!
Issue
The reason we're capturing the Stdout is because that is how the LSP communicates with VScode. At the moment it is hardcoded into
crates\p4-analyzer\src\stdio.rs, which is why the current commented out implementation in Tester uses a Cargo called gag. This crudely redirects the Stdout which enables us to test the LSP output. There are two blaring issues with the current implementation (which is why it's commented out):Cargo testuses--nocaptureand running in serial (--test-threads=1)stdio.rs. This was done incorrectly as it requiredp4-analyzerto depend on thetester!Proposal
I can think of three solutions to the issue: Refactor
stdio.rsso that Stdout isn't hardcode, properly implement an event listener to removetesterdependency, useCommand::new("cargo").arg("run").Refactor
stdio.rsto changetrait WritesourceThis solution would remove the need for capturing the Stdout and instead redirect the message writing source in
stdio.rsdirectly. Is a sensible solution to the issue but do we want to rewrite production code just for testing purposes? Would be a very specific change that might only benefitstdio.rsunit tests and cause head-aches else where!Properly implement an event listener
This solution is already tested to have worked however with the issues stated above. Doing an event listener properly would remove the dependency of
testerinp4-analyzercargo but still require the use of gag cargo.Use
Command::new("cargo").arg("run")Would solve both of the issues but create a new one in that debugging code would be harder. The code would act as a black box only interacting through inputs and outputs!
Note
None of these solutions are perfect and welcome to any ideas!