xPackage should allow some option to not validate installation until after reboot

[wasabii]

Line 982+ of MSFT_xPackageresource.psm1:

if($Ensure -eq "Present")
{
    $productEntry = Get-ProductEntry $Name $identifyingNumber $InstalledCheckRegHive $InstalledCheckRegKey $InstalledCheckRegValueName $InstalledCheckRegValueData
    if(-not $productEntry)
    {
        Throw-TerminatingError ($LocalizedData.PostValidationError -f $OrigPath)
    }
}

This happens immediately after the package has installed. Whether a reboot has been requested or not. Some packages do not complete their installation until after a reboot. An example is .Net 4.6.1. The registry keys that one can use to determine whether it is installed or not do not actually appear until after the reboot.

So, xPackage fails. Even though it has succeeded. There needs to be some way to avoid this, allowing the reboot to proceed, and reevaluation to occur when it comes back up.

[PlagueHO]

Sorry about the delay on this one.

This should be possible by simply calling return after the line:
$global:DSCMachineStatus = 1
at:
https://github.com/PowerShell/xPSDesiredStateConfiguration/blob/dev/DSCResources/MSFT_xPackageResource/MSFT_xPackageResource.psm1#L1021

This check will still fire after the machine is rebooted anyway in Test-TargetResource the next time around and should succeed at that point.

@iainbrighton, can you think of any reason we shouldn't do this?

[iainbrighton]

@PlagueHO We could change line 1024 from an if to elseif? That would ensure that the remainder of the method runs, but a product check only happens if there is no pending reboot. Returning is more explicit about what's going on, however we miss the remaining verbose messages..

[PlagueHO]

@iainbrighton - good thinking! I'll try and submit a PR for this over the next week - although unit and integration tests should be added at the same time as making any other changes (which makes this a much bigger job).

[iainbrighton]

@PlagueHO Yeah - every time I look at fixing an issue it normally means there are a whole load of test that need authoring. It wouldn't be so bad if the tests were already there and a few failing test cases needed adding.. 😢. I guess we'll get there eventually.

[kwirkykat]

I have in-box unit tests for Package that will be ported as part of #160 within the next week

[mbreakey3]

Has this issue been resolved? It doesn't look like it has, but if the only change is to switch the 'if' to an 'elseif' around resetting the DSCMachineStatus, then I can add this fix in while I'm cleaning Package.

[PlagueHO]

Hi @mbreakey3 - No, I don't think it has. I meant to do it, and the fix itself is trivial, but I don't like to submit a fix without having tests to validate something else hasn't broken. I meant to create the tests to be safe making this change but just haven't got round to it. Sorry about that. 😢

[iainbrighton]

@mbreakey3 @PlagueHO I now (finally) have some time to work on #221 and can pick up on this at the same time?

[mbreakey3]

@iainbrighton - That would be great! Thank you!
@PlagueHO - no worries at all! Just wondering if this had been addressed yet.

[PlagueHO]

@iainbrighton - that would be awesome! 😁