xScheduledTask: Added multiple parameters (Fixes #34 and #41) BREAKING CHANGE

[nyanhp]

This pull request describes a major overhaul of the resource xScheduledTask. It is now possible to create all usual trigger types: Once, Daily, Weekly, AtLogon, AtStartup with their respective parameter sets. Additionally it is now possible to set a random interval for all trigger types.
All possible task settings apart from the maintenance settings have been implemented and can be set for all schedule types.
The Test-TargetResource function now implements a function from the module CommonResourceHelper called Test-DscParameterState, to make testing the 30+ properties easier. For this purpose, the module CommonResourceHelper has been added the functions Remove-CommonParameter, Test-DscParameterState and Test-DSCObjectHasProperty.
I realise that this PR introduces some major changes, but it was necessary since the old resource only provided a bare minimum of task settings.

---

This change is 

[msftclas]

@nyanhp,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

[msftclas]

@nyanhp, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

[PlagueHO]

Awesome work @nyanhp ! Just some minor tweaks left - hopefully nothing too tricky. Thanks again and sorry this took so long.

---

Reviewed 1 of 2 files at r2, 2 of 3 files at r3.
Review status: all files reviewed at latest revision, 13 unresolved discussions.

---

Tests/Integration/MSFT_xScheduledTask.Config.ps1, line 4 at r8 (raw file):

{
    Import-DscResource -ModuleName xComputerManagement
    node "localhost"

Please convert to single quotes - same for all Configurations. Node, TaskName, TaskPath and ActionExecutable.

---

Tests/Integration/MSFT_xScheduledTask.Config.ps1, line 61 at r8 (raw file):

            RepeatInterval = [datetime]::Today.AddMinutes(15)
            RepetitionDuration = [datetime]::Today.AddHours(8)
            AllowStartIfOnBatteries =             $true

Nitpick: Remove extra spacing between = and $true

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 30 at r8 (raw file):

        
        #region Schedule type once
        Context '[Once] No scheduled task exists but it should' {

Not a requirement here at all, just a suggestion, but there does seem to be a lot duplicated code here that could be made into a simple loop. E.g.

$configsToTest = @(
@{ configName = 'xScheduledTaskOnceAdd',contextDescription = '[Once] No scheduled task exists but it should' }
@{ configName = 'xScheduledTaskOnceMod',contextDescription = '[Once] A scheduled task exists with the wrong settings' }
...
@{ configName = 'ScheduledTaskStartupDel',contextDescription = '[AtStartup] A scheduled task exists with the wrong settings' }
)

foreach ($configBeingTested in $configsToTest)
{
  Context $configBeingTested.Description {
    $CurrentConfig = $configBeingTested.configName
    ... rest of test here ...
  }
}

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 31 at r8 (raw file):

        #region Schedule type once
        Context '[Once] No scheduled task exists but it should' {
            $CurrentConfig = "xScheduledTaskOnceAdd"

Please convert to single quotes (repeat for all $CurrentConfig)

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 32 at r8 (raw file):

        Context '[Once] No scheduled task exists but it should' {
            $CurrentConfig = "xScheduledTaskOnceAdd"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)

Convert from positional parameters to named parameters (and next line) - repeat for all.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 33 at r8 (raw file):

            $CurrentConfig = "xScheduledTaskOnceAdd"
            $ConfigDir = (Join-Path $TestDrive $CurrentConfig)
            $ConfigMof = (Join-Path $ConfigDir "localhost.mof")

Please convert to single quotes

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 35 at r8 (raw file):

            $ConfigMof = (Join-Path $ConfigDir "localhost.mof")
            
            It "should compile a MOF file without error" {

Please change to single quotes (repeat for all) and also use this text (it is the updated one from the template):

It 'should compile and apply the MOF without throwing' {

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 37 at r8 (raw file):

            It "should compile a MOF file without error" {
                {
                    . $CurrentConfig -OutputPath $ConfigDir

I could be wrong, but this doesn't look right. The usual pattern to compile a MOF from a dot sourced Config is to use:

& $CurrentConfig -OutputPath $ConfigDir

Dot sourcing the file doesn't seem like it should work. But I could be wrong 😁

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 41 at r8 (raw file):

            }
            
            It "should apply the MOF correctly" {

Please convert to single quotes (repeat for all)

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 47 at r8 (raw file):

            }
            
            It "should return a compliant state after being applied" {

Please convert to single quotes (repeat for all)

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 375 at r8 (raw file):

finally
{
    #region FOOTER

It looks like these tests are destructive (e.g. will change the system) - which is OK for integration tests - but it doesn't seem to be cleaning up afterwards if anything fails.

---

Tests/Unit/MSFT_xScheduledTask.Tests.ps1, line 41 at r8 (raw file):

            Context "No scheduled task exists, but it should" {
                $testParams = @{
                    TaskName = "Test task"

Please convert to single quotes (and following lines).

---

Tests/Unit/MSFT_xScheduledTask.Tests.ps1, line 50 at r8 (raw file):

                Mock Get-ScheduledTask { return $null }

                It "should return absent from the get method" {

Please convert to single quotes (and all following It blocks) - sorry, I know this isn't code you created, but we're trying to get all resource modules up to meet minimum standards (which is a big job) 😁

---

Comments from Reviewable

[nyanhp]

Review status: all files reviewed at latest revision, 13 unresolved discussions.

---

Tests/Integration/MSFT_xScheduledTask.Config.ps1, line 4 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes - same for all Configurations. Node, TaskName, TaskPath and ActionExecutable.

Done.

---

Tests/Integration/MSFT_xScheduledTask.Config.ps1, line 61 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Nitpick: Remove extra spacing between = and $true

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 30 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Not a requirement here at all, just a suggestion, but there does seem to be a lot duplicated code here that could be made into a simple loop. E.g.

$configsToTest = @(
@{ configName = 'xScheduledTaskOnceAdd',contextDescription = '[Once] No scheduled task exists but it should' }
@{ configName = 'xScheduledTaskOnceMod',contextDescription = '[Once] A scheduled task exists with the wrong settings' }
...
@{ configName = 'ScheduledTaskStartupDel',contextDescription = '[AtStartup] A scheduled task exists with the wrong settings' }
)

foreach ($configBeingTested in $configsToTest)
{
  Context $configBeingTested.Description {
    $CurrentConfig = $configBeingTested.configName
    ... rest of test here ...
  }
}

I am still growing accustomed to Pester, thanks for the hint. Will consider it!

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 31 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes (repeat for all $CurrentConfig)

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 32 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Convert from positional parameters to named parameters (and next line) - repeat for all.

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 33 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 35 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please change to single quotes (repeat for all) and also use this text (it is the updated one from the template):

It 'should compile and apply the MOF without throwing' {

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 37 at r8 (raw file):

. $CurrentConfig -OutputPath $ConfigDir
Nah, this works fine. But for readability I will convert it to an ampersand

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 41 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes (repeat for all)

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 47 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes (repeat for all)

Done.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 375 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

It looks like these tests are destructive (e.g. will change the system) - which is OK for integration tests - but it doesn't seem to be cleaning up afterwards if anything fails.

Will try to add some cleanup for it, thanks for the reminder!

---

Comments from Reviewable

[nyanhp]

Review status: 5 of 8 files reviewed at latest revision, 13 unresolved discussions.

---

Tests/Unit/MSFT_xScheduledTask.Tests.ps1, line 41 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes (and following lines).

Done.

---

Tests/Unit/MSFT_xScheduledTask.Tests.ps1, line 50 at r8 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Please convert to single quotes (and all following It blocks) - sorry, I know this isn't code you created, but we're trying to get all resource modules up to meet minimum standards (which is a big job) 😁

Done.

---

Comments from Reviewable

[PlagueHO]

Really great job @nyanhp! Excellent work on the integration tests!

---

Reviewed 3 of 3 files at r9.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Tests/Integration/MSFT_xScheduledTask.Integration.Tests.ps1, line 37 at r8 (raw file):

Previously, nyanhp (Jan-Hendrik Peters [MSFT]) wrote…

. $CurrentConfig -OutputPath $ConfigDir
Nah, this works fine. But for readability I will convert it to an ampersand

Cool! All good - just wasn't familiar with the syntax.

---

Comments from Reviewable

[PlagueHO]

@zjalexander or @mbreakey3 - this one

Could one of you confirm and merge when/if you have the time?

[mbreakey3]

Just to confirm, is this a breaking change? I know a lot was added, but was anything changed that will invalidate old configurations of this resource? If so, could you just add (BREAKING CHANGE) to the title of your PR

[nyanhp]

Hi @mbreakey3 , it is a breaking change. The ScheduleType 'Minutes' did not make sense any longer. The interval in minutes or hours can be specified for each ScheduleType. Before my change, the only task type that could be created was of the type Once.
RepeatInterval became a DateTime from which the property TimeOfDay is used as the interval, so this will definitely break existing configurations (As in [datetime]3, in case the old interval was e.g. 3 minutes, will result in an interval of 00:00:00).
The added properties should not break stuff, as they are set to their OS default which is used anyway.

[mbreakey3]

@nyanhp - no problem - can you jst add (BREAKING CHANGE) to the title of this PR so we know to update the version when we publish?

[msftclas]

@nyanhp, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

[PlagueHO]

Hi @nyanhp - this PR has some merge conflicts that need to be fixed.

[nyanhp]

Hi @PlagueHO , resolved the conflicts, thanks!

[PlagueHO]

Reviewed 1 of 1 files at r10.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

README.md, line 123 at r10 (raw file):

### Unreleased
* Updated resources
  - xScheduledTask: Added nearly all available parameters for tasks

Are you able to add BREAKING CHANGE: to the beginning of this line?

---

Comments from Reviewable

[PlagueHO]

Hi @nyanhp - I think there is just one last tweak to be made and this can be merged.

[nyanhp]

Review status: 7 of 8 files reviewed at latest revision, 1 unresolved discussion.

---

README.md, line 123 at r10 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Are you able to add BREAKING CHANGE: to the beginning of this line?

Done.

---

Comments from Reviewable

[PlagueHO]

---

Reviewed 1 of 1 files at r11.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[PlagueHO]

@kwirkykat , @zjalexander - are you able to merge this one? I'm not yet a maintainer of this repo 😁

[kwirkykat]

@PlagueHO Just sent the invite. You should be able to merge now 🙂

[PlagueHO]

Thanks @kwirkykat ! Really appreciate it 😁

[PlagueHO]

Great job @nyanhp - all merged!