On Mon, Sep 17, 2012 at 7:39 PM, Alex Harvey <alexharv...@gmail.com> wrote: > Hi all, > > I've spent the day reading through the Wiki on writing tests > (http://projects.puppetlabs.com/projects/puppet/wiki/Development_Writing_Tests) > and doing the RSpec tutorial that was linked, and otherwise examining rspec > tests in ./spec/unit/util/uptime_spec.rb in facter. I am working on a fix > and have recently updated the following bug: > http://projects.puppetlabs.com/issues/13535. > > I have established that the existing RSpec test relating to who -b is > flawed, and I've come to the conclusion that the who -b calls are probably > unused, no matter what OS is used. Of course, I can't get access to > absolutely every OS, but I've certainly ruled out a pretty large set. > Nonetheless because I am highly risk averse I would like to discuss before > proposing to completely remove the calls to who -b. :) >
The OpenBSD man page doesn't mention it (http://www.openbsd.org/cgi-bin/man.cgi?query=who&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html). > My conclusion is that the method uptime_who_dash_b could only work in theory > on one OS, namely Linux, and that in the case of Linux, it will never be > called, because Linux is catered for by the method uptime_proc_uptime. > Furthermore, I have established that the RSpec test of the who -b > functionality is flawed, based on the contents of the fixture file > who_b_boottime, and works only because only the case of testing one hour > into the future was considered. The test would have failed if any time > between Jan 1 and Aug 1 had been considered, based on the contents > > The fixture file appears to contain output from probably OSX or OpenBSD. > Probably OSX since that is what most of the Puppet Labs employees use. > $ cat ./spec/fixtures/unit/util/uptime/who_b_boottime > reboot ~ Aug 1 14:13 > > In the case of OSX, and other BSD derivatives, the code will use the call to > sysctl -n kern.boottime and never reach the problematic who -b call. That > said, I don't have access to OpenBSD to check but comments in the RSpec > tests tell me that OpenBSD goes to sysctl -n kern.boottime. > > In the case of Linux, this who -b code does work, but the code is redundant, > because Linux uses the uptime_proc_uptime method instead. > > In the case of all other OSes I know of, the who -b code will fail, because > Linux seems to be the only OS that provides the year in who -b output. > > Linux > > $ who -b > system boot 2012-08-27 14:41 > > OSX/NetBSD/PCBSD > > $ who -b > reboot ~ Jul 28 16:34 > > AIX > > $ who -b > . system boot Jun 27 12:05 > > Solaris > > $ who -b > . system boot May 31 17:42 > > HP-UX > > $ who -b > . system boot Aug 13 08:05 > > $ who -b > . system boot Aug 7 13:40 > > Now, next problem is that in the case of Solaris prior to Solaris 8, > uptime_kstat also won't work because kstat wasn't introduced until Solaris > 8. Nonetheless, the fix for AIX proposed by Malcolm Howe will work on > Solaris 10 as well. Thus, it seems simpler and easier if we let Solaris use > the same solution as AIX & HP-UX. > > So I am proposing the following changes as a fix to the bug I am working on > - > > - remove the who -b code altogether, including the associated RSpec test. > - add a case statement to make it explicit as to which OS uses which method What is the advantage of this over just checking what commands are available? My fear with something like this is that as the list of kernels changes this will get out of sync. It also seems to be that the kernel isn't really the determining facter about how to get uptime, rather the available command is. > - add a method uptime_uptime per Malcolm Howe's suggestion that will work > for all commercial Unix. > - remove the uptime_kstat altogether, including the associated RSpect tests > as it will no longer be needed. > - restructure the RSpec tests to follow the new code structure* > > * I am aware of TDD, although it seems the existing tests really follow the > implementation? > > i.e. > > Thus, instead of > > def self.get_uptime_seconds_unix > uptime_proc_uptime or uptime_sysctl or uptime_kstat or uptime_who_dash_b > end > > I would replace this with > > def self.get_uptime_seconds_unix > case Facter.value(:kernel) > when "Linux" > uptime_proc_uptime > when "FreeBSD", "OpenBSD", "NetBSD", "GNU/kFreeBSD", "DragonFly", > "Darwin" > uptime_sysctl > when "SunOS", "AIX", "HP-UX" > uptime_uptime > else > nil > end > end > > I have access to all of these platforms to test on except for OpenBSD and > DragonFly. ' > > I'd like some feedback to make sure I'm not being too aggressive in my plan > for changes here. > > Of course, another solution that involves fewer changes is just do exactly > what Malcolm Howe suggested. > > Thanks in advance, > Alex Harvey > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To view this discussion on the web visit > https://groups.google.com/d/msg/puppet-dev/-/Zj0P4fHpEUMJ. > To post to this group, send email to puppet-dev@googlegroups.com. > To unsubscribe from this group, send email to > puppet-dev+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com. To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
