Merge PSHOrg cAppPool to xWebAppPool

[ghost]

Merging all the community work done here https://github.com/PowerShellOrg/cWebAdministration on the cAppPool resource with the xWebAppPool resource. Thanks @PowerShellOrg

This is all the cAppPool code as of today. The only thing added from xWebAppPool was the State option ("Started"/"Stopped")

I have tested this against 2008r2 and 2012r2.

[msftclas]

Hi @zuehlaa, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[vors]

It's a big change, we would need original author of the resource review it.

[ghost]

Not sure if you mean the original MS author or the author of the changes. @smurawski committed the initial big change and then a few of us added on to it. xWebsite is going to be the same way. It barely resembles the original. With these changes you can now pretty much configure any setting there is. I'll try to get the xWebsite PR in tomorrow so that it can start to be reviewed.

[vors]

Yes, I was talking about MS authors. It would be great if @smurawski will take a look as well.

[vors]

@mhendric can you, please review this changes?

[smurawski]

@zuehlaa @vors @mhendric I would not take this change until we can wrap some tests around it. I was working on some tests in the community fork, but haven't gotten very far. I know there aren't currently any tests for the x resource and this is a major impediment to new development. Given how big a change this is, I think tests are a reasonable requirement.

[TravisEz13]

@smurawski Agreed, we need a level of testing to be included before we can take the code.

[vors]

Adding to the list of 'c' to 'x' merge PRs PowerShell/DscResources#27

[tysonjhayes]

So who or what are we waiting on for this? @smurawski to finish wirting the tests in the c resource? Or someone to write some tests for the x resource?

[smurawski]

Don't block on me, if someone else wants to get tests in place - go for it!

[vors]

@zuehlaa would you be able to include tests?

[ghost]

I haven't used Pester before, but I can give it a shot

[vors]

We would much appreciate it. Don't hesitate to ask me any questions.

[ghost]

I've added a couple initial tests. Just want to make sure I'm on the right track before doing more.

[ghost]

I'm going to stop now :) until you guys let me know that this look ok or not.

[ghost]

@vors Appveyor keeps throwing

start-WebAppPool : Access is denied. (Exception from HRESULT: 0x80070005 (E_ACCESSDENIED))
At C:\projects\xwebadministration\DSCResources\MSFT_xWebAppPool\MSFT_xWebAppPool.psm1:343 char:21
+                     start-WebAppPool -Name $Name
+                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (IIS:\apppools\PesterAppPool:String) [Start-WebAppPool], UnauthorizedAccessException
    + FullyQualifiedErrorId : InvalidOperation,Microsoft.IIs.PowerShell.Provider.StartAppPoolCommand

Do I need to do something different? The tests seems to work on my local machines.

[tysonjhayes]

Couple of thoughts.

1. Totally works on my machine too. I'm not sure why it's throwing access denied.
2. Could you break up and group some of the tests together with a Context? For example. Group all Test-TargetResource calls together like so.

Context 'Test-TargetResource' {
    It 'Totally does everything I want' {
        $true | should be $true
   }
}

This way it breaks up the tests visually when running the Pester tests.

[ghost]

I broke up the tests into 2 contexts and made it much easier to add new tests.

It 'Passes autoStart Test when same' {             
    TestTargetResourceSame "autoStart" "add.autoStart" | Should be $true
}

It 'Fails autoStart Test when different' {              
    TestTargetResourceDiff "autoStart" "add.autoStart" "true" "false"| Should be $false
 }

It 'Should set autoStart Test ' {             
    SetTargetResource "autoStart" "add.autoStart" "true" "false"| Should be $true
}

Not sure why it showed up as unknown on the commit and not me.

[KarolKaczmarek]

@TravisEz13 Can you take a look at this PR and the test failures?

[nzspambot]

Guys is this going to go anywhere? I also want to add some stuff around providers and autostart when this pull is merged

[tysonjhayes]

Hi @zuehlaa - are you still working on this? It's been a while since we've seen any activity on this. If you are not working on this anymore (or I don't hear from you) I'm going to fork this and try to fix it up myself as we'd really like some of this functionality. Please let me know.

[ghost]

I've been waiting to hear back from @KarolKaczmarek or @TravisEz13 before spending more time on it

[tysonjhayes]

Hi @zuehlaa could you resolve the merge conflicts and repush this so we can address whatever the current failures are? At this point this code is months out of date so we'll need to get it updated before we can being to address any of the problems.

[ghost]

Crap, that didn't work right

[ghost]

Not sure why all those extra file changes showed that shouldn't be there. Nor do I know how to clean this up. Should I close and start fresh with a new PR?

[tysonjhayes]

Yeah I'm pretty lost by what's going on with that.

[ghost]

See #63

[tysonjhayes]

Cool - I'm going to close this so we don't get confused. Thank you so much for following up on this.

[ghost]

Nice, I like that. I will get that done.