MockPromptForString + doctor to work with ValidatePathError. #325
Conversation
…ed to insert the future answers and it will load them as aa user answer when a prompt is being done
| /** | ||
| * A class which used for testing and which mocks user's input. | ||
| */ | ||
| export class MockPromptForStrings implements Prompt { |
There was a problem hiding this comment.
I just realised this isn't actually "for strings" anymore - it can just be called MockPrompt now.
There was a problem hiding this comment.
But the messages are always strings
There was a problem hiding this comment.
I had thought that the "ForStrings" was about the return type of the methods - for example that promptInput would always return a string (as opposed to a Color or something). The version we've got now works if it's used in a place that returns a Color.
There was a problem hiding this comment.
The "forStrings" was for the input we accept, and as for now it's only strings
There was a problem hiding this comment.
I had the same impression about T always being a string, which affects the validateFunction input and the Promise<T> output. But T can now have any values and we could drop ForStrings. The fact that addMessages() is always a string seems to be more of an implementation detail?
| const nextResponse = this.responses[0]; | ||
| this.responses.shift(); |
There was a problem hiding this comment.
Any reason why we're not just calling shift() and assigning the return value to nextResponse?
| const nextResponse = this.responses[0]; | |
| this.responses.shift(); | |
| const nextResponse = this.responses.shift(); |
There was a problem hiding this comment.
Yup, if I do as you say, I get an error because the value could be undefined or string.. Even if I check that it isn't empty before.. this is the only way I could make this work
| /** | ||
| * A class which used for testing and which mocks user's input. | ||
| */ | ||
| export class MockPromptForStrings implements Prompt { |
There was a problem hiding this comment.
I had the same impression about T always being a string, which affects the validateFunction input and the Promise<T> output. But T can now have any values and we could drop ForStrings. The fact that addMessages() is always a string seems to be more of an implementation detail?
| import {Result} from '@bubblewrap/core'; | ||
| import {MockPromptForStrings} from './mock/MockPromptForStrings'; | ||
|
|
||
| async function validationFunction(message: string): Promise<Result<string, Error>> { |
There was a problem hiding this comment.
Should we test with a validationFunction that returns something other than a string?
There was a problem hiding this comment.
I don't see how it could change the bottom line.. every case depends on the implementation but with those tests I made sure that the logic of the class is good and functioning (At least I think so)
…oved 'ForStrings' part)
Added 'MockPromptForString' and changed 'doctor' to work with 'ValidatePathError'.