puppetlabs/facter — pull request #429 by jeffmccune (2013-04-16 02:34:12+00:00)
Title: (#20236) Refactor the virtual fact and fix dmidecode

Without this patch there are a ridiculous number of problems with the
virtual fact. This patch reduces the number of problems to something
more along the lines of an absurd number of problems.

The major problem this patch addresses is that a single local variable,
`result`, is used to store the determined-thus-far value of the virtual
fact. Sometimes this value is checked to see if it's still at the
initial value of 'physical'. Only sometimes, however, most of the time
subsequent platform checks blindly overwrite the value even if we've
already made a determination.

This patch addresses the problem by using the `next` keyword to return a
value from the block passed to `setcode` as soon as a value is
determined. Only if all attempts have failed do we resort to returning
'physical' This patch completely eliminates the use of the `result`
accumulator that is constantly being overwritten.

As part of this change in behavior, the bug described in #20236 is
resolved because we no longer fail to check `dmidecode` if the output of
`lspci` does not result in a determination.

Surprisingly, the spec tests pass with almost no modification, the one
exception being an unexpected invocation on a mocked object resulting
from a confinement check added by this patch. This leads me to believe
this is a true refactor, or as close to one as we're going to get.

The patch does change the behavior substantially. If multiple results
could be determined without this patch applied then Facter would return
the value of the last determination. With this patch applied Facter
will return the first determined result. It's unclear which behavior is
expected of the system, the spec tests do not express an expected
behavior in this scenario. The comments at the top of virtual.rb lead
us to believe the first result should be returned, implying this patch
gets us a little closer to correct behavior. I suspect there will be
issues filed regardless of which way we go. There certainly have been
no end to the number of issues filed against the virtual fact thus far.