BIND 10 #2713: b10-cmdctl-usermgr's prompts should be revisited

[BIND 10 Development]

#2713: b10-cmdctl-usermgr's prompts should be revisited
-------------------------------------+-------------------------------------
            Reporter:  muks          |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  cmd-ctl       |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130305
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:10 jinmei]:
 >
 > Comments on the revised branch: all minor, but there are some non
 > trivial number of them, so we'll probably need another round of cycle.
 >
 > '''b10-cmdctl-usermgr_test'''
 >
 > - I'd make new_getpass "private"
 > {{{#!python
 >         self.new_getpass = new_getpass
 > }}}
 >   same `PrintCatcher.orig_stdout_write` for attributes for
 >   `OverrideInput`, `TestUserMgr.usermgr_module`,
 >   `TestUserMgr.usermgr` etc.
 >

 done

 >
 > - `TestUserMgr.run_check`: parameter stdin isn't documented.
 >

 actually I though I was going to use that functionality, but didn't end up
 doing it, so I removed the parameter again

 > - test_default_file: I don't understand this comment:
 > {{{
 >         Check that the prompting methods do verification
 > }}}
 >

 that is right, because it makes no sense :) removed, and updated the
 docstrings of the two next ones

 > - test_prompt_for_password_different: maybe a matter of taste, but you
 >   can avoid `nonlocal` if you define getpass_different_called as
 >   object's attribute (self.getpass_different_called) (and in that case
 >   I'd also make it "private").

 TBH i prefer the explicit nonlocal in this case (and keep scope to this
 specific function)
 i guess making it a dict would work as well (called = { "times": 0 }) but
 then we'd really be getting dirty :p

 > - test_prompt_for_password_different: also a matter of taste, you can
 >   use '+= 1'  here:

 doh of course

 > - test_prompt_for_password_empty: same comments apply.
 > - test_prompt_for_password_empty: you could avoid variables options
 >   and args.

 ahyes sorry, forgot that one

 > - test_prompt_for_user: same comments about nonlocal apply.

 and the same answer does as well ;)

-- 
Ticket URL: <http://bind10.isc.org/ticket/2713#comment:12>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development