feat(plugin): use stdio instead of tcp socket for transport #66
Conversation
jaspervdveen
changed the title
| */ | ||
| private log(level: string, message: string, ...labels: string[]): void { | ||
| // Strip trailing newlines from the message | ||
| const cleanedMessage = message.replace(/\n+$/, ''); |
There was a problem hiding this comment.
Why would we remove newlines? Let's make the logging output recognizable to the end user (like what they already recognize in the 'normal' output).
| /** | ||
| * Abstract interface for mutation server transport implementations | ||
| */ | ||
| export interface ITransport { |
There was a problem hiding this comment.
It is considered a bad practice to prefix interfaces with an I in TypeScript, as there is no technical need to do so (in C#, there is, because there is no visual distinction when extending interfaces vs classes).
But this interface isn't even needed, since it is the same as StdioTransport.
| }; | ||
| write(data: string | Buffer) { | ||
| if (!this.#process?.stdin) { | ||
| throw new Error('Process stdin is not available'); |
There was a problem hiding this comment.
This is unreachable code, since we're not overriding the stdio options. stdin should always exist, or Node.js isn't working correctly. I would be happy using a ! here.
| onPartialResult: (partialResult: MutationTestResult) => void, | ||
| ): Promise<MutationTestResult> { | ||
| const subscription = this.notifications | ||
| const subscription = this.transport.notifications |
There was a problem hiding this comment.
So we're subscribing to the notifications whenever we start a new run. What happens when the user begins multiple runs? Won't the notifications interfere with each other?
Resolves #58
BREAKING CHANGE: drop support for socket transport with mutation server