[MERGE] Add a get_login method to UIFactory and test get_password

[John Arbash Meinel]

Vincent Ladeuil wrote:
>>>>>> "Martin" == Martin Pool <mbp at canonical.com> writes:
> 
>     Martin> On 19 Feb 2007, Vincent Ladeuil <v.ladeuil+lp at free.fr> wrote:
>     >> I needed to write tests involving querying user for login and
>     >> password.
>     >> 
>     >> I rewrote part of the tests in the process so that they can be
>     >> run from a dumb terminal (I get tired of
>     >> test_text_factory_prompts_and_clears failures).
> 
>     Martin> That sounds reasonable.  Does anything call these
>     Martin> already or are they new?
> 
> get_password is used for ftp, ssh and http. get_login is needed to
> address bug #72792. I tried to isolate that patch to ease review.
> 
>     >> Unicode experts comments welcome.
>     >> 
> 
>     >> One point I couldn't decide was: should the login and
>     >> passwords be encoded ?
> 
>     Martin> I'm not sure.  I think your position is a reasonable
>     Martin> place to start.
> 
> Let's start with that then.
> 
> <snip/>
> 
>     >> """See UIFactory.nested_progress_bar()."""
>     >> raise NotImplementedError(self.progress_bar)
>     >> 
>     >> +    def get_login(self, prompt='', **kwargs):
>     >> +        """Prompt the user for a login (generally on a remote host).
>     >> +
>     >> +        :param prompt: The prompt to present the user
>     >> +        :param kwargs: Arguments which will be expanded into the prompt.
>     >> +                       This lets front ends display different things if
>     >> +                       they so choose.
>     >> +
> 
>     Martin> Rather than passing the prompt and kwargs to these
>     Martin> methods why not let the caller combine them?
> 
> I think the original intent in get_password (which I duplicated)
> was to "let different front ends display different things". I'm
> +0.5 on this until I see useful examples.
> 
> Note that get_boolean add a '[y\n] ?' and that get_login and
> get_password add a ': ' at the end of the prompt which seems
> appropriate for CLI.
> 
> But that's a different point than kwargs though.

That was the original intent. It means that the string being used is
relatively fixed, so if you want to replace it with something else
because of a GUI, it is easy to do, while inspecting an already expanded
string is difficult.

At this point it was a bit of "future proofing" that hasn't done
anything yet, so it may not be the best way to go about it. But since
ui_factory was meant to be a way to change the interface from CLI to
something else, it seemed reasonable.

To me, the alternative is to make the interface more specific and not
allow a prompt at all. Rather the prompt is defined by the function. So
we would have "get_password_for_user_at_host()" (which may just be
get_password()).


...

v- I'm concerned about this being tested on multiple platforms, since it
seems pretty OS specific. What would happen on Windows, Mac or FreeBSD?

Also, if we do something like this, it might be better if we exposed a
fake file descripter (tempfile.mkstemp comes to mind).


> +class FakeStdin(StringIOWrapper):
> +    """Simulated stdin for tests only.
> +
> +    We pretend to be the real stdin by redirecting the fileno method so that
> +    getpass.getpass can succeed changing the echo mode of the real
> +    stdin. More precisely, getpass change the echo mode via tcsetattr which
> +    requires a file descriptor, once the user have entered its password the
> +    echo mode is restored (this is garanteed by a try-finally). So basically
> +    the risk for the tester is to lose ist echo if he attemps to type some
> +    characters *while* the echo is disabled.
> +
> +    That allows tests to can user inputs without having to implement a
> +    full-fledged stdin.
> +    """
> +
> +    fileno = sys.stdin.fileno
> +
> +    def __init__(self, string, encoding='ascii'):
> +        StringIOWrapper.__init__(self, string.encode(encoding))
> +        self.encoding = encoding
> +
> +

John
=:->