Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for unit testing via Unix OS #309

Merged
merged 5 commits into from
Apr 19, 2024

Conversation

chambersmp
Copy link
Contributor

@chambersmp chambersmp commented Apr 12, 2024

Summary

Adds cross-platform unit testing support for resources implemented by the dsc_base_provider.

  1. Allow dsc_base_provider.ps_manager to use pwsh on non-Windows.
  2. Hardcode path delimiter to ':' for splitting Unix $PATH system variable to determine pwsh_path.
  3. Update Pwsh::Utils.on_windows? to reliable identify the OS running the Ruby interpreter to correctly identify if pwsh or Powershell should be used for dsc_base_provider.canonicalize.
  4. Update dsc_base_provider spec tests to validate .ps_manager using pwsh_path and pwsh_args for non-Windows usage

Additional Context

Root Cause

  1. The provider dsc_base_provider is hardcoded to use Windows PowerShell paths instead of the platform agnostic pwsh which forces unit testing on non-Windows OS to fail waiting for a PowerShell process to respond.
# Output from MacOS with pwsh installed
> pdk test unit --puppet-version=7 -v
...
Failures:

  1) test_class on windows-2019-x86_64 - test compilation is expected to compile into a catalogue without dependency cycles
     Failure/Error: raise "Failure waiting for PowerShell process #{@ps_process[:pid]} to start pipe server"
     
     RuntimeError:
       Failure waiting for PowerShell process 13743 to start pipe server
     # ./spec/fixtures/modules/pwshlib/lib/pwsh.rb:167:in `initialize'
     # ./spec/fixtures/modules/pwshlib/lib/pwsh.rb:61:in `new'
     # ./spec/fixtures/modules/pwshlib/lib/pwsh.rb:61:in `instance'
     # ./spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb:1080:in `ps_manager'
     # ./spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb:257:in `invoke_dsc_resource'
     # ./spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb:358:in `invoke_get_method'
     # ./spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb:63:in `block in canonicalize'
     # ./spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb:50:in `collect'
     # ./spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb:50:in `canonicalize'
     # ./spec/classes/test_class_spec.rb:69:in `block (4 levels) in <top (required)>'

After Fix:

  1. When dsc_base_provider is configured to use pwsh when Pwsh::Utils.on_windows? returns false, compilation completes successfully on Unix.
  • pwsh must be installed locally on the OS running the spec test.
# Output from MacOS with pwsh installed
 pdk test unit --puppet-version=7 -v
pdk (INFO): Using Ruby 3.2.2
pdk (INFO): Using Puppet 7.29.1
[✔] Preparing to run the unit tests.
/opt/puppetlabs/pdk/private/ruby/3.2.2/bin/ruby -I/Users/user1/.pdk/cache/ruby/3.2.0/gems/rspec-core-3.13.0/lib:/Users/user1/.pdk/cache/ruby/3.2.0/gems/rspec-support-3.13.1/lib /Users/user1/.pdk/cache/ruby/3.2.0/gems/rspec-core-3.13.0/exe/rspec --pattern spec/\{aliases,classes,defines,functions,hosts,integration,plans,tasks,type_aliases,types,unit\}/\*\*/\*_spec.rb --format documentation
Run options: exclude {:bolt=>true}

test_class
  on windows-2019-x86_64 - test security_options
    is expected to contain Dsc_securityoption[Accounts: Administrator account status] with dsc_accounts_administrator_account_status => "Enabled"
  on windows-2019-x86_64 - test users
    is expected to contain User[my_user] with ensure => "present", home => "C:\\Users\\my_user" and managehome => true
  on windows-2019-x86_64 - test execs
    is expected to contain Exec[services] with command => "get-service" and provider => "powershell"
  on windows-2019-x86_64 - dsc_lite
    is expected to contain Dsc[iis] with resource_name => "WindowsFeature", module => "PSDesiredStateConfiguration" and properties => {"ensure"=>"present", "name"=>"Web-Server"}
  on windows-2019-x86_64 - test compilation
    is expected to compile into a catalogue without dependency cycles

Code coverage
  must cover at least 95% of resources


Coverage Report:

Total resources:   5
Touched resources: 5
Resource coverage: 100.00%

Finished in 5.81 seconds (files took 6.65 seconds to load)
6 examples, 0 failures

Thought process behind the implementation.

  1. pwsh must be installed on the Unix OS running the spec test as this is a dependency for compilation.
  2. $PATH system variable must contain a reference to the $PSHOME (pwsh install path) to allow the provider to dynamically find the executable via the Pwsh::Manager.pwsh_path method.
  3. DSC resources implement a canonicalize feature during catalog compilation (Depends on pwsh)
  • Usually canonicalize is implemented entirely within the Ruby without external process calls. The dsc_base_provider has been configured to make external process calls to PowerShell which currently fail on Unix OS.
  • the implementation of canonicalize aims to prevent corrective changes for case-mismatch by comparing user supplied values (manifest) and machine current state (invoke-dscresource -method Get) during catalog compilation.
    • If the same resource within the manifest is found on the target node, and the values are the same except for case-mismatch, the dsc resources current state values will be compiled into the catalog instead of the manifest values to prevent flapping
    • During compilation, the canonicalize implementation for dsc_base_provider uses an external PowerShell process via a named pipe to query the dsc resources on the target node (i.e agent or local OS executing unit test).
    • When a compile.with_all_dependencies test is run, the PowerShell process attempts to query the local hosts state (i.e. start PowerShell process on Unix OS looking to get current state of Windows DSC resources declared within the rspec context).
    • As PowerShell paths are not present on Unix nodes, the Ruby process will timeout waiting for PowerShell.
    • To allow a non-Windows OS to compile, the dsc_base_provider should use pwsh instead of PowerShell.
      • This allows the local host to start a pwsh process and run invoke-dscresouce -method Get for each resource defined in the manifest.
      • As the local OS will have no DSC resources managed, the method will return nil for current state.
      • The DSC resources within the manifest will be compiled uncontested.
  1. When running a unit test on non-Windows to compile Windows DSC resources, the File constant File::PATH_SEPARATOR used to separate values of $PATH to find pwsh incorrectly uses ';' instead of ':'.
  • File::PATH_SEPARATOR => ';' because the context is a Windows OS test which prevents splitting of the paths in $PATH resulting in failure to find pwsh
  • To prevent this, the delimiter for splitting non-windows $PATH system variable has been hardcoded to ':' (colon).
  • This may be problematic if any non-Windows nodes delimit $PATH with anything other than ':'.
  • An alternative may be to add a before block to mock Pwsh::Util.on_windows? => false to force the spec tests tof follow the non-Windows conditional logic similar to the updated spec tests for the ruby-pwsh.
    • This alternative requires the user to configure mocks which may result in pain for end users. Updating the Util method to return the hosting OS may result in better user experience.
    • Alternative Debunked The Pwsh::Util.on_windows? mock does not persist for compile.with_all_deps and therefore cannot force Unix hosts to take this path. It is overwritten on compilation resulting => true.
require 'spec_helper'

describe 'test_class' do
  on_supported_os.each do |os, os_facts|
   before do
     # Force on_windows? to return false to use pwsh and split $PATH via ':'
     allow(Pwsh::Util).to receive(:on_windows?).and_return(false)
   end
  context "on #{os} - test compilation" do
      let(:facts) { os_facts } 
        it do   
          is_expected.to compile.with_all_deps
        end
    end
end
  1. When running unit tests on a non-Windows OS, the Ruby interpreter follows the Windows logic causes compilation issues.
  • Pwsh::Utils.on_windows? returns true regardless of running on Windows or Unix when assessing a Windows context in rspec.
  • This method has been modified to correctly identify which OS is running the Ruby interpreter, instead of the OS within the rspec context to ensure
    • usage of pwsh over PowerShell on Unix
    • set $PATH delimiter to ':' instead of ';' on Unix

Related Issues (if any)

Resolves #308

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified.

@CLAassistant
Copy link

CLAassistant commented Apr 12, 2024

CLA assistant check
All committers have signed the CLA.

@chambersmp chambersmp added enhancement New feature or request feature labels Apr 12, 2024
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.01%. Comparing base (fec0326) to head (82fd9ca).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #309      +/-   ##
==========================================
+ Coverage   91.98%   92.01%   +0.03%     
==========================================
  Files           6        6              
  Lines         711      714       +3     
==========================================
+ Hits          654      657       +3     
  Misses         57       57              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chambersmp chambersmp force-pushed the feature_unix_rspec_support branch 5 times, most recently from 8dd7630 to 4db604a Compare April 15, 2024 01:29
@chambersmp chambersmp force-pushed the feature_unix_rspec_support branch from 4db604a to 56a4b81 Compare April 15, 2024 01:41
@chambersmp chambersmp marked this pull request as ready for review April 15, 2024 01:59
@chambersmp chambersmp requested a review from a team as a code owner April 15, 2024 01:59
Copy link
Contributor

@jordanbreen28 jordanbreen28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chambersmp This is amazing work. I've left what is really just a small suggestion rather than anything else. Other than that this LGTM

lib/pwsh/util.rb Outdated Show resolved Hide resolved
@jordanbreen28 jordanbreen28 removed the enhancement New feature or request label Apr 15, 2024
@jordanbreen28 jordanbreen28 merged commit 63335e2 into puppetlabs:main Apr 19, 2024
8 checks passed
@chambersmp chambersmp deleted the feature_unix_rspec_support branch April 22, 2024 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants