Hi, I'm using ubuntu. I haven't set username and password in git options, neither in environment. when I upload a patch to bugzilla (with webkit-patch upload), I'm asked if I want to store my username and password in the system keyring. I say yes, and the credentials are stored. But those credentials are not used next time I use webkit-patch upload.
That's because in Credentials.read_credentials, we try to read password from keyring before prompting for username. Instead, if username was not read from environment, git, or keychain, we could prompt for username, and try to read password from keyring after.
Created attachment 155020 [details] patch proposal
Comment on attachment 155020 [details] patch proposal Seems reasonable to me. Is it possible to write a test for this?
(In reply to comment #3) > (From update of attachment 155020 [details]) > Seems reasonable to me. Is it possible to write a test for this? I don't really known how to write a test for that: if we want to create unittest, at some point, the User.prompt() method will block them. Is there a way to override User.prompt() with a mock object ?
Yes. If you use a MockHost object, the User object will be mocked out for you by default.
You can grep for "MockUser" to see how it gets used. Many existing tests use it.
Created attachment 155337 [details] updated patch with test
Comment on attachment 155337 [details] updated patch with test View in context: https://bugs.webkit.org/attachment.cgi?id=155337&action=review > Tools/Scripts/webkitpy/common/net/credentials.py:136 > + def read_credentials(self, User=User): the host has a user field, so you should be able to just say self.host.user.prompt(), and then down in your test you can create a credentials object, pass it a mock host, and override the prompt method. That should cut down on the scaffolding a fair amount.
(In reply to comment #8) > (From update of attachment 155337 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=155337&action=review > > > Tools/Scripts/webkitpy/common/net/credentials.py:136 > > + def read_credentials(self, User=User): > > the host has a user field, so you should be able to just say self.host.user.prompt(), and then down in your test you can create a credentials object, pass it a mock host, and override the prompt method. That should cut down on the scaffolding a fair amount. In Credentials, host is just a string
Comment on attachment 155337 [details] updated patch with test View in context: https://bugs.webkit.org/attachment.cgi?id=155337&action=review >>> Tools/Scripts/webkitpy/common/net/credentials.py:136 >>> + def read_credentials(self, User=User): >> >> the host has a user field, so you should be able to just say self.host.user.prompt(), and then down in your test you can create a credentials object, pass it a mock host, and override the prompt method. That should cut down on the scaffolding a fair amount. > > In Credentials, host is just a string ugh, you're right. Okay, your approach is fine for now until we can clean that up. this object shouldn't be creating executives and users directly. one nit: parameters should be lowercased, so user=User.
Created attachment 155383 [details] same patch; typo fixed
Comment on attachment 155383 [details] same patch; typo fixed thanks!
Comment on attachment 155383 [details] same patch; typo fixed Clearing flags on attachment: 155383 Committed r124140: <http://trac.webkit.org/changeset/124140>
All reviewed patches have been landed. Closing bug.