dsccommunity / SharePointDsc

The SharePointDsc PowerShell module provides DSC resources that can be used to deploy and manage a SharePoint farm
MIT License
247 stars 107 forks source link

[SPFarm] Ensure that flights are registered in SharePoint Subscription #1428

Closed Yvand closed 1 year ago

Yvand commented 1 year ago

Pull Request (PR) description

SharePoint Subscription introduced flighting. When creating a farm using PowerShell, one new step is to run cmdlet Update-SPFlightsConfigFile to ensure that all flights in current build are registered in the configuration database.

This Pull Request (PR) fixes the following issues

In SharePoint Subscription, ensure that all flights available in current build are registered in the configuration database

Task list


This change is Reviewable

ykuijs commented 1 year ago

The SPSE unit test failed. I think because the Get-SPDscRegistryKey function isn't mocked and therefore doesn't return any value. This is causing the Join-Path to fail. Please create proper mocks for Get-SPDscRegistryKey, Test-Path and Update-SPFlightsConfigFile.

ykuijs commented 1 year ago

This last change broke the unit tests for the other versions as well 😉 Looks like something is not going well here. You can run the tests locally, by just running the tests ps1 file.

I see that you have implemented an if statement in the code. It is also possible to create multiple mock using ParameterFilters. That way you can mock the same function multiple times, once for each scenario.

Yvand commented 1 year ago

Indeed, it is because I should Mock Update-SPFlightsConfigFile only if current environment is SharePoint SE.

I already thought about it but I didn't know how to do it, but I also didn't know about ParameterFilters, hopefully it will allow me to fix that, I'll take a look asap

ykuijs commented 1 year ago

You can check the used version number like we do here: https://github.com/dsccommunity/SharePointDsc/blob/fdb9a4a15b8ca78f291a612efcd8774dfd8816aa/tests/Unit/SharePointDsc/SharePointDsc.SPDistributedCacheService.Tests.ps1#L427-L428

codecov[bot] commented 1 year ago

Codecov Report

Merging #1428 (9d17ae8) into master (b90202b) will decrease coverage by 1%. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1428   +/-   ##
======================================
- Coverage      84%     84%   -1%     
======================================
  Files         145     145           
  Lines       22647   22657   +10     
======================================
+ Hits        19076   19084    +8     
- Misses       3571    3573    +2     
Impacted Files Coverage Δ
...PointDsc/DSCResources/MSFT_SPFarm/MSFT_SPFarm.psm1 90% <100%> (+<1%) :arrow_up:

... and 1 file with indirect coverage changes