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

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

Merged
merged 6 commits into from
Apr 22, 2023

Conversation

Yvand
Copy link
Contributor

@Yvand Yvand commented Apr 17, 2023

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

  • Added an entry to the change log under the Unreleased section of the file CHANGELOG.md.
    Entry should say what was changed and how that affects users (if applicable), and
    reference the issue being resolved (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof and comment-based
    help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@Yvand Yvand requested a review from ykuijs as a code owner April 17, 2023 13:46
@Yvand Yvand changed the title Yvand/spfarm add flighting [SPFarm] Add support for registering flights in SharePoint Subscription Apr 17, 2023
@Yvand Yvand changed the title [SPFarm] Add support for registering flights in SharePoint Subscription [SPFarm] Ensure that flights are registered when running SharePoint Subscription Apr 17, 2023
@Yvand Yvand changed the title [SPFarm] Ensure that flights are registered when running SharePoint Subscription [SPFarm] Ensure that flights are registered in SharePoint Subscription Apr 17, 2023
@ykuijs
Copy link
Member

ykuijs commented Apr 18, 2023

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
Copy link
Member

ykuijs commented Apr 21, 2023

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
Copy link
Contributor Author

Yvand commented Apr 21, 2023

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
Copy link
Member

ykuijs commented Apr 21, 2023

You can check the used version number like we do here:

if ($Global:SPDscHelper.CurrentStubBuildNumber.Major -eq 16 -and
$Global:SPDscHelper.CurrentStubBuildNumber.Build -gt 13000)

@codecov
Copy link

codecov bot commented Apr 21, 2023

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%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Member

@ykuijs ykuijs left a comment

Choose a reason for hiding this comment

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

LGTM

@ykuijs ykuijs merged commit 580a33f into dsccommunity:master Apr 22, 2023
10 checks passed
@Yvand Yvand deleted the yvand/spfarm-add-flighting branch April 23, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants