Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to write unit tests around Prompts #40

Closed
seancarroll opened this issue Nov 29, 2021 · 6 comments
Closed

How to write unit tests around Prompts #40

seancarroll opened this issue Nov 29, 2021 · 6 comments

Comments

@seancarroll
Copy link

Is your feature request related to a problem? Please describe.
I just started to play around with this as part of a CLI i'm building and was curious what suggestions you had for being able to unit test logic that contained inquire prompts? I see that internally you use prompt_with_backend

Describe the solution you'd like
I would like to be able to add unit tests around code that uses inquire prompts. One possible solution would be to expose prompt_with_backend

Describe alternatives you've considered
Not sure about alternatives but would love to hear your suggestions

Additional context

@mikaelmello
Copy link
Owner

Hi @seancarroll!

That's right, I managed to write a few tests and actually planned to expose this API sometime in the future, but the reason I didn't until now was because it still makes tests very dependent on the current implementation. Each prompt handles almost raw key events as opposed to "commands" (e.g. handles Key::DownArrow instead of Command::MoveDown). Back then I was still planning to improve this part, allowing this custom backend to re-map any desirable key bindings.

I am totally okay in prioritizing this next development for you, as I don't have any other major plans for this project at the moment. But to be really honest, I'll start a new job abroad in 1 month and I'm reserving this time I have left here to enjoy my family, so I'm not sure how much I'll be able to contribute for now.

If you're interested, we can discuss more this direction and I'd be very happy to accept contributions!

@mikaelmello
Copy link
Owner

We can also consider exposing the current method as-is in an alpha release or something, it if makes your life easier sooner :)

@seancarroll
Copy link
Author

@mikaelmello this isn't an immediate need for me so happy to discuss direction and options. Given you mentioned "commands" I started some very preliminary research and stumbled up enigo which they say is a cross platform input simulation in Rust. Perhaps this is a direction worth looking into?

@mikaelmello
Copy link
Owner

@seancarroll that is an awesome find and it would be a great addition for E2E tests in the future, but for unit tests I was thinking about something else, which I honestly don't know if it is indeed better.

For context, each Terminal has a method to convert a key input from their respective exported enums to inquire's own unified Key enum. https://github.com/mikaelmello/inquire/blob/main/src/ui/key.rs#L15-L33. Each Prompt then updates its state based on the last Key pressed.

The current problem (at least for me) is that key binding definitions are made very deep in the code, not clear from a first glance. This is a problem for users in general (what if we break the API without noticing) and for tests that shouldn't have to worry at this low level of implementation.

I was wondering if it was worth for each prompt to have something like:

pub enum MultiSelectCommand {
  MoveUp,
  MoveDown,
  ToggleSelection,
  ...
}

impl From<Key> for MultiSelectCommand { ... }

IMO, we have the following pros and cons:

Pros:

  • Easier to reason about the behavior of the key bindings in a certain prompt, we centralize this definition
  • Allows us to write tests that do not have to worry about keys input, only programatically changing state.

Cons:

  • Adds more complexity in the overall logic of a prompt.
  • Might not work well with the current way I used to process keys input when the prompt has a "text box".
    • When processing a key input, prompts with a text box (here called Input) catch any keys relevant to the prompt itself and then call the Input's key handler to process any bindings related to processing text, such as writing a letter in the text box or pressing backspace to delete something.
    • We would need to add a catch-all or something in the <Prompt>Command, such as MultiSelectCommand::Other(Key), and then, when calling the Input's key handler, we would get the inner key to convert to something like TextBoxCommand. Looks a little bit ugly.

Finally, I believe we should also add E2E tests to ensure we don't break our lib's external API, and enigo looks like the path forward.

@seancarroll
Copy link
Author

@mikaelmello, thanks for the explanation and insights.

@mikaelmello
Copy link
Owner

I want to improve this crate's testability for both the crate and its users. I have created #71 to track the developments on this part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants