BIND 10 #2172: Add support for OS X in b10-showtech

[BIND 10 Development]

#2172: Add support for OS X in b10-showtech
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jinmei
                       Type:  task   |                Status:  reviewing
                   Priority:         |             Milestone:
  medium                             |  Sprint-20120821
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  Core
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  3      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:5 jinmei]:
 > '''sysinfo.py'''
 >
 > - (although not specific to this branch) I don't think it a good
 >   practice to have Linux specific notions like mem_cached,
 >   mem_buffers, or get_platform_distro() at the top level.  It seems to
 >   be a typical example of the "the world is Linux" mindset.  IMO we
 >   should either introduce a higher abstraction for these that can
 >   apply more system independently or move them to Linux specific code.
 >

 We can go for a higher-abstraction layer (or maybe even a different kind
 of abstraction, by defining how to get things and which things to get for
 which operating systems). But to keep the changes reasonable, I changed it
 a little bit differently; values set to None are simply not printed.


 > - The implementation seems to share much of the code with the FreeBSD
 >   version.  I'd unify them as much as possible based on the DRY
 >   principle.
 >

 done

 > - at least on my MBP kern.smp.active isn't available:
 > {{{
 > % sysctl kern.smp.active
 > second level name smp in kern.smp.active is invalid
 > }}}
 >   and I don't even know if it's possible to activate/deactivate an SMP
 >   support on MacOS X.
 >

 one of the things no longer printed (OSX is not the only one where it has
 no meaning)

 > - I don't think it a good idea to have a hardcoded page_size
 >   default.  If it's not known (which should be very rare or indicate
 >   we need to update the parser script), IMO we should also leave
 >   parameters that depend on it unknown.
 >

 Done

 > - Likewise (and although it's not specific to the MacOS version), I'd
 >   generally avoid using magic numbers like "-1" to represent
 >   "unknown", "unavailable", etc, because magic numbers make the intent
 >   of the code more vague and can often lead to unexpected bugs (e.g.)
 >   as a result of automatic conversion.  Python has a perfection
 >   representation for this type of concepts: None.  I'd have
 >   get_mem_free, etc return None when the value is unknown and convert
 >   it to corresponding string at the main program, or we could simply
 >   have get_xxx return the corresponding string if we only use them for
 >   printing purposes.
 >

 Changed all defaults that were no strings to None

 > - The `_mem_free` value doesn't match the corresponding "Free" size
 >   shown in Activity Monitor.  It looks like we should use the sum of
 >   'Pages free' and 'Pages speculative' instead of only the former.
 >   See also:
 http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/osfmk/mach/vm_statistics.h
 >

 fixed

 > - hw.physmem doesn't match the actual available memory, at least on my
 >   MBP.  hw.memsize seems to be the one that should be used.  See:
 >   http://stackoverflow.com/questions/583736/determine-physical-mem-size-
 programmatically-on-osx
 >

 sigh :) Fixed

 > '''sysinfo_test.py'''
 > - `_my_osx_os_sysconf`: what's the magic number of 91?  Arbitrary
 >   chosen?  As a magic number hater, I'd rather define a constant
 >   variable with commenting the intent and use the variable throughout
 >   the file.  Also, using the same constants for different systems is
 >   probably not a good idea anyway as we wouldn't be able to catch some
 >   type of regression.
 >

 Done

 > - `_my_osx_subprocess_check_output`: this generally seems to be the
 >   same as that for FreeBSD.  If we unify the main code (see above),
 >   it's probably better to unify the test case, too.  Or, if we want to
 >   separate the two test cases (which is not necessarily a bad idea),
 >   it's probably better to use different faked constants.
 >

 Done, check_output is now roughly following the same hierarchy as the
 sysinfo classes themselves. Did the same for the actual testing code

 > - `_my_osx_subprocess_check_output`: maybe related to the previous
 >   point, 'daemon' doesn't seem to be very compatible with OS X.
 > {{{#!python
 >     if command == ['hostname']:
 >         return b'daemon.example.com\n'
 > }}}
 >

 Since these are now in one place, changed it to 'test.example.com'

 > - test_sysinfo_osx: I've fixed a trivial typo in a comment.  And, this
 >   seems to suggest we might rather unify these test_xxx's and switch
 >   from it depending on the system type, rather than repeating the same
 >   logic for every case.
 >

 Oh right just mentioned I had done this.

 > - test_sysinfo_osx: not really specific to this branch, but it'd be
 >   safer to restore the original functions in tearDown().

 Also done.

 And while I was at it, I also fixed another big annoyance; it had all this
 code to fake system call results, then ran only the tests for the current
 operating systems. Changed it to always run all tests on all systems (had
 to add one more fake system call to do that).

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