[MERGE] Add get_username() call to the UIFactory.
Vincent Ladeuil
v.ladeuil+lp at free.fr
Wed Apr 1 07:37:10 BST 2009
>>>>> "Jelmer" == Jelmer Vernooij <jelmer at samba.org> writes:
Jelmer> Thanks again for the quick review; I've fixed the issues you raised,
Jelmer> but I'm not sure about the last one.
Jelmer> On Tue, Mar 31, 2009 at 05:14:39PM +0200, Vincent Ladeuil wrote:
>> >>>>> "Jelmer" == Jelmer Vernooij <jelmer at samba.org> writes:
Jelmer> + """
Jelmer> + prompt += ': '
Jelmer> + prompt = (prompt % kwargs)
Jelmer> + self.prompt(prompt)
Jelmer> + return self.stdin.readline().rstrip("\n")
>> get_non_echoed_password can raise NotATerminal and encode the
>> prompt according to the terminal encoding, I think get_username
>> should do the same, time to refactor...
Jelmer> I think that's because get_non_echoed_password() uses getpass.getpass().
Jelmer> NotATerminal's error message specifically mentions "password"
class NotATerminal(BzrError):
_fmt = 'Unable to ask for a password without real terminal.'
Yes, it also mentions that it will not be able to ask for
it. What makes you think you will be able to ask for a username
if you can't ask for a password ?
Jelmer> and get_boolean() works fine without it.
Good catch, it should be fixed too then :)
Jelmer> Is there any reason why we should raise that
Jelmer> exception even if it wouldn't cause issues?
Backward compatibility. We may need to rethink how we want to
handle situations where authentication cannot be achieved without
a terminal, but until then, I don't want to break existing cases.
Jelmer> TestUIFactory provides a custom implementation of
Jelmer> get_non_echoed_password(); making it provide a mock
Jelmer> implementation of get_username() that doesn't raise
Jelmer> NotATerminal seems kind of pointless as we would only
Jelmer> be testing the mock implementation, not the actual
Jelmer> one.
That's a different issue.
If you mean NotATerminal is tested nowhere, you've found yet
another hole in our test suite, feel free to provide more tests
but I don't require them at that point (a FIXME: will do :)
Vincent
More information about the bazaar
mailing list