This updates the CI build with two new features:
1. **Validation against Windows PS7 (PowerShell Core)**
This pipeline previously had three platforms that it tested against:
* `Windows PS5`
* `Linux PS7 (PowerShell Core)`
* `macOS PS7 (PowerShell Core)`
With this change, we add a fourth platform: `Windows PS7 (PowerShell Core)`
2. **Build queuing**
The unit tests operate against live GitHub accounts (as opposed to mocking out the execution of the API calls). Each platform has its own account that it operates against to alllow each platform to be tested in parallel. However, if more than one build is queued at once, then the builds can start to stomp over the expected state in the tests accounts. This change adds a new `job` to the pipeline which will create a "queue" of running builds by only allowing a build to continue processing once all previously queued builds have completed.
Initial idea came from here:
https://developercommunity.visualstudio.com/idea/365730/prevent-parallel-execution-of-the-same-build-defin.html
- Fixes#239
#221 reported that this repo causes PSScriptAnalyzer to sporadically throw an error.
In order to report the full error needed for analysis, report the full stack trace. In order to make this easier, I've updated `Invoke-ScriptAnalyzer` to run within a `try/catch using -ErrorAction Stop`, and report any error found with `$_.Exception.StackTrace` before re-throwing the error.
In a previous commit (b4439f4a6b), the three platforms in the CI pipeline (Windows, Linux, and Mac) were modified to execute serially since they were all operating against the same shared test account, which caused the Pester tests to operate unstabily.
I've now created multiple test accounts so that that each platform can operate in isolation of each other. I've updated the UT template to accept parameter input, and modified the CI (and release) pipeline to reference a different AccessToken/OwnerName/OrganizationName based on which platform is being targeted.
Additionally:
* Had to fix some tests to keep everything passing as well;
* Two Contents tests started failing again because GitHub once again changed the HTML output for the standard generated README.md file. This time around, I've attempted to make the tests more robust to future changes.
* Two RepositoryForks tests were unreliable in the way that they were written, so they were updated as well to be more robust when being executed in a parallel UT environment (since we can't control when a fork for this repository is being created/removed).
* Updates the pipelines to always use the most recent OS images.
* Fixes some invalid links on some of the Azure DevOps badges in the README
This will now convert the output of `Invoke-ScriptAnalyzer` into an NUnitXml file that can then be published through the CI system so that we can fail on ScriptAnalyzer failures, as well as easily
see them in the test output.
Got the initial idea for this approach from @MathieuBuisson's [blog post](https://mathieubuisson.github.io/psscriptanalyzer-first-class-citizen/) (which directly converted the ScriptAnalyzer results into an NUnit XML file, but only had a single passing test for the success scenario). I then combined that idea with one from a [Dr. Scripto post](https://devblogs.microsoft.com/scripting/psscriptanalyzer-deep-dive-part-3-of-4/) which used Pester to invoke PSScriptAnalyzer, ensuring that there was a test created for each possible Rule. The problem with the Dr. Scripto approach was that multiple failures of the same rule still resulted in a single failure, and the test output lost the actual message from ScriptAnalyzer.
I ended up implementing this differently than Mathieu, directly working with the XML class to build up the XML document, but really appreciated the initial ideas that he had with this approach.
Sample build test result when there was a Script Analysis failure: https://dev.azure.com/ms/PowerShellForGitHub/_build/results?buildId=84441&view=ms.vss-test-web.build-test-results-tab
There were a number of issues going on:
* Needed to force Pester `v4.10.1` for the install because `5.0` was just released and
has some breaking changes to investigate.
* Pester was failing to save its output because the output directory
didn't exist. Without that, it couldn't actually publish the results.
* The `ciOrganizationName` pipeline variable had a typo in it, which meant
that we were passing an empty string as the `OrganizationName` to tests
that needed it
* We needed to ensure consistent line endings for `Tests/Config/Settings.ps1`
so that the hash generated on all platforms (Windows/mac/Linux) would be
the same.
* GitHub changed the default HTML generated for README.md which was causing
two RepositoryContents tests to fail.
* Needed to make sure that the three platforms (Windows/mac/Linux) run the
unit tests _serially_ instead of in parallel, because they each modify the shared
state of the same account, and when running at the same time they were
stomping over each other and causing erroneous failures.
Resolves#182
Somehow a number of PSScriptAnalyzer issues snuck in. This fixes them.
The main PSScriptAnalyzer issues needing to be fixed were:
* `PSReviewUnusedParameter` - This one came up a lot due to our heavy
usage of `Resolve-RepositoryElements` and `Resolve-ParameterWithDefaultConfigurationValue`
which both end up referencing their parameters by grabbing them off
the stack. That means that `NoStatus` and `Uri` are frequently
never directly referenced. So, exceptions were added. There were
two cases (in GitHubAnalytics) where there was a false positive due
to PowerShell/PSScriptAnalyzer#1472
* `PSUseProcessBlockForPipelineCommand` - We had a number of functions
that took pipeline input, but didn't actuall use the `process` block.
This actually caught a bug with `Group-GitHubIssue` and
`Group-GitHubPullRequest`. Added correct `process` block usage for
most of the functions, but removed pipeline support for those where
it didn't actually make sense anymore.
* `PSUseDeclaredVarsMoreThanAssignments` - These are false positives
in the Pester tests due to the usage of `BeforeAll`. There wasn't
an obvious way to use `SuppressMessageAttribute` in the Pester test,
so I used a hacky workaround to "use" the variable in the `BeforeAll`
block. I could have added the suppression to the top of the file,
but I still want to catch real issues in those files later.
* `PSAvoidOverwritingBuiltInCmdlets` - It turns out that there's a bug
with PSDesiredStateConfiguration in PS Core 6.1.0 where it was exporting
internal functions. This was thus a false-postive flag for Write-Log.
See PowerShell/PowerShell#7209 for more info.
Also, it turns out that `Group-GitHubPullRequest` hadn't actually been
exported, so I fixed that too.
A CI check is failing because this value is wrong. Updating it to be correct.
Oddly enough, it still gets the wrong value on Linux and macOS for some reason.
Until that is understood, stop checking that on macOS and Linux,
During UT's, `Reset-GitHubConfiguration` and `Clear-GitHubAuthentication` are called frequently.
This change moves their warning messages to be in Verbose output instead, in order to reduce
the noise that is commonly seen when they're called.
Additionally, the warning that is seen at the start of running a UT test suite is now
completely suppressed if the user has modified `Tests/Config/Settings.ps1` (presumably to
have it reference accounts that they have access for). This is achieved by storing the hash of the Settings file in the code, along with documentation to remind users to update it if the Settings
file is ever permanently changed.
Finally, an additional test has been added to the CI pipeline to validate that the Settings file
content matches the hash that is present in the code.
* Refactors the CI build yaml so that there can be a release build
that benefits from the same template content.
* Updates the documentation for releasing updated modules.
* Refreshes the list of contributors to the project.
Howard Wolosky
Christoph Bergmeister [MVP]
Tyler James Leonhardt
Steven Maglio