Remove complex scripts from AppVeyor.yml files

[BrianFarnhill]

Right now we have some more PowerShell scripts getting a bit more complex in the appveyor.yml file (I know SharePointDsc is a great example of that since I generate all my doco and stuff like that at the build server). The problem here is that while these scripts are in the AppVeyor.yml file we miss out on things:

• Syntax highlighting
• Script Analyzer warnings and details
• Intellisense when editing

My proposed solution is to take these scripts out and put them in to a PSM1 module file that we can load up and just call the cmdlets that we include there from the Appveyor file. I have an example of this in a feature branch I'm working on right now:

• Appveyor.yml
• .appveyor\appveyor.psm1

So you can see here we dramatically simplify what goes in to the appveyor YML file and move the rest of the script out to a separate file that allows us a way better editing experience. In theory we could also write some pester tests over this to make sure that our appveyor scripts always do the things we expect them to do (more useless when run locally than on Appveyor I think, but helpful all the same).

Thoughts on this anyone? Worth looking at implementing something like this on all other DSC modules?

[kwirkykat]

@BrianFarnhill There were thoughts here of putting a module just like this in the DSCResource.Tests repo since that one already has to be downloaded by the appveyor.yml file. Then all the resource repositories would have mostly common code, and we wouldn't have to make changes in every repository when we want to make a change to the appveyor files

[PlagueHO]

@BrianFarnhill - I agree with @kwirkykat. Adding the module DSCResource.Test would seem to be a good solution. Although it would be a little counter intuitive given the name of DSCResource.test.

[BrianFarnhill]

@kwirkykat @PlagueHO I'm actually a little hesitant to put it in to the main repo there as what we do with each build may vary slightly. Sure I know I do more in mine that other modules do, but even things like how we inject build numbers can mean some slight changes might be needed. That said - we could probably come up with a way to put a base bit of functionality in and then have a model that allows individual repos like the ones I run to hook in to it for different things as needed. Do you think that approach could work? Where everyone uses the base one and then the modules that need to do more have a point to inject more code?

[PlagueHO]

@BrianFarnhill - Just to confirm, I'm talking about this Repo rather than the main DSCResources repo.

But I'd think that if you implemented the functions in this module then it would be up to the individual repos to use them or now. If they didn't use them then nothing changes. Eventually, each repo could be moved across to implement your new AppVeyor functions as the benefits become too good to pass up 😁

[PlagueHO]

Another alternative to this is to create a new repo for this AppVeyor.psm1 module. For example DSCResource.AppVevor. We'd need to parameterize the functions in this module - which wouldn't be hard. @mbreakey3, @kwirkykat - do you have any preference/thoughts on this? If we were to create a new repo, this would be something that would need to be done by your team.

I'd be happy to submit a PR with the content based on @BrianFarnhill 's code if helpful. I'm eager to get this working as Brian has added some pretty cool features to the AppVeyor build process and manually adding this code to each and every repo seems so 2015 😁

[kwirkykat]

@PlagueHO
I'm not convinced a new repo is necessary.
Especially since the first step for this new AppVeyor module would always be downloading DscResource.Tests. It makes more sense to me to just include them together in the same repo/ downloadable module.
And it fits with the theme of DscResource.Tests since AppVeyor is what we use to do all of our automated testing.

Why do you think the appveyor module should be separate from the common tests?

BAHAHAHA just saw the 2015 comment 😆

@BrianFarnhill You can still call whatever you want in the appveyor.yml file after calling Invoke-AppVeyorInstallStage, Invoke-AppVeyorTestStage, etc. This module would just be for the common bits of each step. And as @PlagueHO mentioned we could add parameters for anything in that common code that you want to be different for SharePointDsc.

On top of that, we could have a standardized way that appveyor code specific to a module could be called, so you could have your .appveyor/appveyor.psm1 file in SharePointDsc and this module could find that standardly-structured/named module and try to call a standardly-named function from that module like Invoke-SharePointDscAppVeyorTestStage (by dynamically inserting the module name) or Invoke-ModuleSpecificAppVeyorTestStage (with no dynamic insertion) or something along those lines.

[PlagueHO]

@kwirkykat - Using DscResource.Tests is good with me - and keeps things simpler. You're right, adding another "git clone" from another repo just makes things more complex.

My only reason for considering a separate repo was just because of the naming of DscResource.Tests won't be an obvious location if you're looking for the some non-test related code (e.g. @BrianFarnhill 's documentation building code and markdown linting etc). But I don't actually think this is a problem worth adding complexity to solve.

I also like your idea about the AppVeyor module "extensibility". This sounds like a really nice solution that enables a standard methodology across all resources (I love standards 😁).

So, would we want to call the new module "AppVeyor.psm1" and would it be in the root folder of the DscResource.Tests?

[BrianFarnhill]

@PlagueHO the doco generating stuff is already in the main DSCResources repo (I added that to what we clone in to the AppVeyor environment in SharePointDsc already) so that's there. The markdown linting I think belongs in the main test one though so that's cool.

In terms of extensibility I think if we set a standard location to look for the scripts I would suggest we put it in a folder called ".appveyor" so we can put anything related to the build in there - the root folders of a lot of our repos are starting to get plenty of other "stuff" in them these days, so use folders to group some of it together makes a bit more sense to me. So if we say ".appveyor/CustomTasks.psm1" with some specific method names in there as the standard, the main script we write that has the standard stuff can look for that and if the script and methods are in there, they can get called from there. Thoughts?

[kwirkykat]

@PlagueHO Yes on AppVeyor.psm1 in DscResource.Tests
@BrianFarnhill Adding an AppVeyor file to the resource modules (if needed) sounds good to me. Why ".appveyor" instead of "AppVeyor"?

[PlagueHO]

@kwirkykat - Cool. @tysonjhayes has just finished implementing this in xNetworking. So that enabled platform to test the module with (I wasn't comfortable using SharePointDsc because I'm not as familiar with it).

I'll aim to submit a PR this weekend. I'm eager to begin converting other larger resources over to use it.

[BrianFarnhill]

@kwirkykat Just following a standard I see around folders that contain "supporting assets" for a project instead of actual code (like .vscode or .github)

@PlagueHO Let me know when you get the PR open, I'm keen to see what you've done with it all.

[PlagueHO]

@BrianFarnhill - cool. I'll try and make sure that whatever I do will be generalized enough to support both xNetworking and SharePointDsc. I think @tysonjhayes pretty much lifted your code as-is (it's even in the .AppVeyor folder) so it should be easy enough to generalize.

[BrianFarnhill]

@PlagueHO @tysonjhayes Love your work boys

[PlagueHO]

@BrianFarnhill - that's because we copied it all from you pal! 😁

[PlagueHO]

Because of the features in this module, only resources that have been updated to have an AppVeyor build process (Wiki etc) and folder structure like that of xNetworking and SharePointDsc will be able to make use of this module (AppVeyor.psm1).

When other resources are updated to meet the structure defined in SharePointDsc they will be able to use this module as well.

Is there any objection to this?

[kwirkykat]

@PlagueHO I'm confused. There shouldn't be any file restructuring needed here.
The only file structure that matters for appveyor is where the tests are which can be passed in as a parameter to whatever 'run tests' function we have.

The AppVeyor.psm1 going into DscResource.Tests should be able to be used by any resource module regardless of its structure because we just install it and run its functions in the appveyor.yml file. Then each module has a .appveyor file at its root to contain any extra functionality that needs to run specific for that module. But only if that .appveyor file exists.

[PlagueHO]

@kwirkykat - SharePointDsc and xNetworking are both restructured quite a lot (moved to subfolders etc) compared to other resources. They also perform tests and other functions using a different process: They load a test harness module and execute a function within that to run the tests. For example:
https://github.com/PowerShell/SharePointDsc/blob/dev/.appveyor/appveyor.psm1#L30

However, I think I can probably make this support the older method as well easily enough (as well as the new methods in SharePoingDsc and xNetworking) if you think that would be a better approach?

[kwirkykat]

@PlagueHO Ah. I see now. It's the test harness and the wiki generation that seems really different. We could add switch parameters like 'UseTestHarness' and 'GenerateWiki' so that we could use it the fancy way (like SharePointDsc and xNetworking) with a test harness and/or wiki generation or the simple way (like xPSDesiredStateConfguration or AuditPolicyDsc) where we just invoke all the tests in the Tests folder. Then when other modules are updated, they can gradually turn on the fancy features.

I'm just a little nervous to accept a common module that only works for 2 repos because I don't have 'restructure 56 repositories' on my to-do list at the moment...
In addition there are repos that may never end up getting updated to the fancier structure.

[PlagueHO]

@kwirkykat - I like your thinking! I was just about to suggest this (or something similar). And I completely understand about not wanting to add tech debt to 56 repos 😁

We can have one or more code paths supported for each function as well as support for a extension by way of ..appveyor\customappveyor.psm1 tasks.

It might be worth having a "Type" parameter in each function that is a validateset:
E.g.

param
(
[ValidateSet('Default','UseHarness')
[String] $Type
)

We could then also have parameter sets that expose the parameters for each "Type". For example the UseHarness Type would require additional parameters such as the path to the harness file and the function to call to execute the tests. If the Type was default then no extra parameters would be required.

This allows us to extend the "Type" to support other future methods without too much hassle.

What do you reckon?

[kwirkykat]

👍 I like it

[PlagueHO]

@kwirkykat - cool! I'll get it done and push it to my fork. I'll then create a branch of xNetworking and of xStorage in my repos and point them at my fork of DSCResource.Tests and make sure they both work. If they do I'll submit the PR etc.

Thanks for helping me get this straight!

[BrianFarnhill]

@PlagueHO I like it too mate, I would also default it to "Default" so that way there is as little as possible to change for the other 50 or so repos.

[PlagueHO]

@BrianFarnhill - absolutely agreed 100%!

[PlagueHO]

Regarding the unit test failures in SharePointDsc, it is possible that this is caused by the new Pester 4.0.2 that was released the a few days ago. It has caused failures in the unit tests in xStorage. I've logged the problem with the pester team: pester/Pester#697

[PlagueHO]

Although I've practically finished with this change, I'm going to pause for a bit and wait to get feed back from the community on some things that would make things a lot simpler: #244 and #180

In summary the things that I'd like feedback on are:

1. Including Markdown validation tests in DSCResources.Tests meta.tests.ps1 will likely cause most repos to fail testing as many that I've seen have a large number of markdown violations.
2. Including Example validation tests in DSCResources.Tests meta.tests.ps1 will likely cause some repos to fail testing.
3. Having shared build, test and deploy code spread across two repos (DSCResources and DSCResources.Tests) is a bit messy and makes things more complicated than they could be. I think it would be better if all shared build, test and deploy code was in the same repo (DSCResources.Tests).

It is a bit of a pity about the naming of DSCResources.Tests - it would be much better if it were names DSCResources.SharedCI or something along those lines - but it is probably a bit late for a change of that nature.

Could definitely use some feedback! 😁

[johlju]

I had some additional thoughts over at #180 that would have been more suitable here. Maybe we shall move the discussion here instead?

I would be a undertaking renaming in all code (tests) in all repos to the new name. So I think that is out. 😄

[tysonjhayes]

@PlagueHO to address your points:

1/2) Most of the PSSA tests are actually commented out to not fail the tests but just provide warnings. I'd suggest we start off with that methodology and agree on a date in which we make them required. That'd allow most of the resources to get updated as they are doing their work.
3) Move it to DscResource.Tests, I never liked having to clone this repo to do work.

As for the name, we could totally just start a new one and use that for all of the breaking changes instead of using the warning system I suggested. Support will be phased out to the old one and deprecated as people move to the new system. Also allows time for any serious rework or breaking changes (if any) that are needed during the change.

[kwirkykat]

@PlagueHO @johlju @tysonjhayes
1/2) Said basically the same thing as Tyson over in #180. But you can turn on the tests for any HQRMs and we can update any problems there quickly
3) Yes. Ideally no one should have to clone this repo ever. I would move any code over to the DscResource.Tests repo. (Though yes, that is not an ideal name for the repo now...)

[PlagueHO]

This has now been implemented in PowerShell/DscResource.Tests#90

@BrianFarnhill - Can we close this issue?

[johlju]

Closing this as solved. There are updated appveyor.yml template which uses minimal code. Please reopen or comment if this was closed in error.