Get unit tests working against Pester 5.x

[HowardWolosky]

Pester 5.0 was just released two days ago, and has since had a successive release. There are some breaking changes with this major release.

This issue is to track the work needed to migrate the tests to work against 5.x, and then update the yaml and documentation to stop requiring 4.10.1.

[giuseppecampanelli]

When I added more testing for GithubRepositories I had initially made it Pester 5.x compliant, therefore I'd like to give this a go

[HowardWolosky]

I should have changed how this was marked earlier. I think that I should have this fixed as a side-effect of the work I did in the tests to support pipelining. I'll be verifying that tonight or tomorrow, but I don't think we need additional help on this one. Thanks for your interest though.

[HowardWolosky]

@themilanfan - It turns out that the work from #242 didn't magically make the tests work with both Pester 4 and 5.
If you're interested in tackling this, it is yours. The expectation coming out of this though is that the tests will work with both 4 and 5. Let me know if you'd still like to take this on. Thanks!

[tigerfansga]

@HowardWolosky i would be willing to take this on, but I don’t think it would be possible to write a test to work on both Pester v4 and v5. v5 has several significant breaking changes that change the structure of a test script.

https://pester-docs.netlify.com/docs/migrations/breaking-changes-in-v5
https://pester-docs.netlify.com/docs/migrations/v4-to-v5

[HowardWolosky]

@HowardWolosky i would be willing to take this on, but I don’t think it would be possible to write a test to work on both Pester v4 and v5. v5 has several significant breaking changes that change the structure of a test script.

Thanks @tigerfansga. I was aware of the breaking changes. I'm not sure why I made that comment earlier expecting that it would support both 4 and 5. Not sure what I was thinking at the time.

The appeal of getting to work with 5 is that I was told that the PowerShell VSCode extension would be able to run individual tests directly in VSCode if migrated to Pester 5 which would be a win.

So, if you're interested, this project would definitely appreciate it (and I'd love to see what I was doing wrong back when I first tried to get it to work).

[tigerfansga]

@HowardWolosky, I worked on one it the test files last night. I’ll push it up to my fork later today and send you a link so you can see.

[tigerfansga]

@HowardWolosky Here is the file I've updated - https://github.com/tigerfansga/PowerShellForGitHub/blob/PesterV5/Tests/GitHubUsers.tests.ps1

When you have a moment, you should look at https://pester.dev/docs/migrations/v4-to-v5 for how the Invoke-Pester command itself is changed as it will affect the CI

[tigerfansga]

@HowardWolosky I've converted a handful of test files. You can go ahead and assign this to me.

[HowardWolosky]

That's great news, @tigerfansga. I took a look at your first change, and it seems like it was a pretty straight-forward change. I hope the rest are as easy.

I'm unable to assign this task explicitly to you since you're not technically a member of the project, but I've changed the Tags to more clearly indicate the current status of things.

[tigerfansga]

Some are harder than others but for the most part they are straightforward. I have run into an issue running the tests. My test account has been flagged. I have a case with support but I'm stuck until the account is unflagged.

[HowardWolosky]

My test account has been flagged. I have a case with support but I'm stuck until the account is unflagged.

That happened with me too a while back with some of the test accounts that are used in the CI pipeline. Support was pretty responsive in getting those unflagged (I believe it was < 24 hours at the time).

In the interim, you can also always create another test account.

[tigerfansga]

I probably won't be able to work much this weekend so I'll see if it's cleared up Monday.

[tigerfansga]

@HowardWolosky Wanted to give you an update on my progress. I'm about 70% complete and hopefully can finish in a week or two.

I have run into an issue with one of the tests. When I start work on a test script, I first validate that I can run it as is with Pester v4 and all the test pass.

For PowerShellForGitHub/Tests/GitHubMiscellaneous.tests.ps1 some the tests for Get-GitHubCodeOfConduct fail. They appear to be failing because Get-GitHubCodeOfConduct appears to not be working.

When I try to run the failing command myself, I get the same results. Can you verify if this command is working properly for you?

PS> Get-GitHubCodeOfConduct -OwnerName 'PowerShell' -RepositoryName 'PowerShell'
Telemetry is currently enabled.  It can be disabled by calling "Set-GitHubConfiguration -DisableTelemetry". Refer to USAGE.md#telemetry for more information. Stop seeing this message in the future by calling "Set-GitHubConfiguration -SuppressTelemetryReminder".
Invoke-WebRequest: C:\Users\mark\Documents\PowerShell\Modules\PowerShellForGitHub\0.16.0\GitHubCore.ps1:301
Line |
 301 |              $result = Invoke-WebRequest @params
     |                        ~~~~~~~~~~~~~~~~~~~~~~~~~
     | {"message":"Not Found","documentation_url":"https://docs.github.com/rest"}

[HowardWolosky]

When I start work on a test script, I first validate that I can run it as is with Pester v4 and all the test pass.

A very smart way to approach the problem.

....
For PowerShellForGitHub/Tests/GitHubMiscellaneous.tests.ps1 some the tests for Get-GitHubCodeOfConduct fail. They appear to be failing because Get-GitHubCodeOfConduct appears to not be working.

When I try to run the failing command myself, I get the same results. Can you verify if this command is working properly for you?

Thanks for the update @tigerfansga! When you mentioned Code of Conduct not working, I knew that sounded familiar. I tried it myself and it failed, and looking it up in the documentation, that specific API (for retrieving the CoC for a specific repo) was nowhere to be seen.

Finally I remembered that I had previously brought this very issue up: #343.

As for next steps for what you are doing, I'd suggest deleting the tests as part of your change -- we won't gain much by simply marking them to be disabled because the tests will simply be different once Get-GitHubCommunityProfileMetrics is eventually implemented.

Thanks for doing this Test migration work and for bringing this issue to attention once again.

So yeah, for now, keep doing what your doing (and thanks!) and just delete those specific CoC tests.

If you encounter other (non-CoC) tests that are failing in the rest of your migration, those should likely be tagged as disabled and then we should get issues opened to track fixing the test and/or API.

Thanks again!

[tigerfansga]

Just wanted to give an update. Summer has has me busy but I've gotten back to this. I have four test files left to update.

I did run into a small issue on Tests/GitHubRepositories.tests.ps1. One of the tests is failing because Get-GitHubRepositoryTeamPermission is returning push when the team has maintain. I haven't looked into it. @HowardWolosky can you verify if this is a true issue or something wrong in my environment?

[HowardWolosky]

Just wanted to give an update. Summer has has me busy but I've gotten back to this. I have four test files left to update.

No worries. Breaks are natural. I really appreciate the effort here.

I did run into a small issue on Tests/GitHubRepositories.tests.ps1. One of the tests is failing because Get-GitHubRepositoryTeamPermission is returning push when the team has maintain. I haven't looked into it. @HowardWolosky can you verify if this is a true issue or something wrong in my environment?

It looks to be [a logic bug in the project, but I'm not sure why we're seeing it now and we weren't before (it wasn't failing a month ago and this logic hasn't changed in over a year). My best guess is that they weren't returning back push permission as true for maintainer until recently.

PowerShellForGitHub/GitHubRepositories.ps1

Lines 3928 to 3935 in 22e3d7b

	elseif ($result.permissions.push)
	{
	$permission = 'push'
	}
	elseif ($result.permissions.maintain)
	{
	$permission = 'maintain'
	}

Those two conditions should be checked in the opposite order, as we should be saying that maintain is the actual permission that has been set if that is set to true but admin isn't.

[HowardWolosky]

I have a fix out for that in #361

[tigerfansga]

@HowardWolosky I'm done converting all the test. I'm running another check with the latest commits. Here is what I have left to do.

1. Fix the formatting of the tests due to the changes from removing the try/catch blocks - unsupported in Pester v5
2. Update the documentation. This one is a little tough because the parameters from Invoke-Pester have changed significantly. I'll look a little more in the coming weeks.
3. Updating the CI/CD scripts. Do you want me to do those or should a maintainer do that?

[tigerfansga]

@HowardWolosky I've run into a new issue. It looks like with the new GitHub Projects, most (if not all) of the apis for projects have changed. As such most of the project related tests are failing. I'm going to move forward on getting a PR submitted

[HowardWolosky]

@HowardWolosky I'm done converting all the test.

Awesome and exciting

Here is what I have left to do. ...
3. Updating the CI/CD scripts. Do you want me to do those or should a maintainer do that?

If you're able / willing to update the CI/CD scripts yourself, that would be greatly appreciated.

@HowardWolosky I've run into a new issue. It looks like with the new GitHub Projects, most (if not all) of the apis for projects have changed. As such most of the project related tests are failing. I'm going to move forward on getting a PR submitted

That's annoying. I guess just disable those tests for the time being and we'll get an Issue opened to track getting the current supported updated against the newer API's. Thanks for the heads-up.

[tigerfansga]

@HowardWolosky I'll go ahead and submit a PR now. I still need to update the docs and I can look at the CI/CD.

[tigerfansga]

The update for the CI was trivial. I changed the required version parameter to 5.3.3