conradkleinespel / rpassword

Cross platform Rust library to read a password in the terminal (Linux, BSD, OSX, Windows, WASM).
Apache License 2.0
244 stars 38 forks source link

Using read_password in tests. #19

Closed DaveLancaster closed 6 years ago

DaveLancaster commented 6 years ago

Thanks for the useful crate. How would you feel about adding something like an Option<Cursor> as a parameter for read_password() so that the input could be easily mocked in tests etc? Happy to submit a PR if something like this would be appropriate (currently building a lib which I would like to use rpassword, so this would be handy for me).

conradkleinespel commented 6 years ago

Hi @DaveLancaster ! Thanks for the idea ! 👍

I get where you're coming from because I'm having a hard time mocking stdin too.

We can have something like read_password_with_reader<T: std::io::Read>(reader: std::io::Read) which I can easily merge without breaking backward compatibility. And then you can use that function instead of read_password and pass it any cursor you'd like in tests.

But. I won't be adding a parameter to read_password() because I don't want to break backward compatibility and the function is meant to be a no-brainer, which it won't be if it has parameters.

If possible, mocking STDIN should be adressed in the standard library itself, because that would make mocking easier for the entire Rust community. What do the folks over at the stdlib team think ?

DaveLancaster commented 6 years ago

Hey Conrad, thanks for the reply. My thought was to rename read_password to something like read_password_with_reader with the Option<Cursor> or whatever, and then just create a read_password stub that called it with None to preserve backwards compatibility & simplicity. I wonder if that might be an acceptable approach?

conradkleinespel commented 6 years ago

@DaveLancaster Sounds good! Why not Option<T: Read> though ? Cursor implements Read and using Read instead of hardcoding Cursor makes thing a bit more flexible.

If you want Cursor, that's fine, but call the function read_password_with_cursor 😉

Quick question: how would you go about testing read_password_with_reader(None)? Up until now I have used the little shell script in tests/tests-unix.sh and done manual testing on Windows, but that takes quite some time 😓

DaveLancaster commented 6 years ago

So thinking about it maybe Option<T: BufRead> is best as it gives you flexibility and provides .read_line().

My thought was that if you used the read_password() shim everything would work as before (including whatever tests you have in place) but if you wanted to use rpassword in a library you could opt to use read_password_with_reader() directly and under certain circumstances mock out STDIN.

To give an example of where this might be useful, I am writing a simple crate to easily ask questions in command line applications. Usage is something like:

let answer = Arsk.input("What should we do today?").prompt(':').ask().unwrap();

To allow me to test some of this I call it in tests like so:

let answer = Arsk.input("What should we do Today?")
    .prompt(':')
    .redirect_input(mock())
    .ask().unwrap();

This works fine, but I was hoping to use rpassword to provide a .no_echo() type implementation, so it would be handy for me to be able to mock the STDIN data in the same way. Hope that vaguely makes sense!

Thanks for your time.

conradkleinespel commented 6 years ago

Sounds good! Go ahead with that!

And just in case you're looking for something that prompts input without no_echo, there is rprompt.

DaveLancaster commented 6 years ago

@conradkdotcom Hey man, thanks for all your help.