maester icon indicating copy to clipboard operation
maester copied to clipboard

Update-MaesterTests removing my folders

Open dafuni opened this issue 9 months ago • 5 comments

I am running the Maester updates and receiving a message that all my folders will be removed. Can I please get a clarification on this? image

dafuni avatar May 13 '24 16:05 dafuni

Ok so I looked into this for you, and I'm not sure if its by design or not but...

The issue is that your not providing a path so it uses your current working directory. now even though there is a cmdlet that gets the maester-tests folder path, that path is not used when getting a list of folders to use. It instead uses your current working directory as your not passing a path.

I've not tested this but a quick work around would be to run Update-MaesterTests -path "C:\Users\{username}\maester-tests"

Let me know how you get on!

DrPye avatar May 13 '24 18:05 DrPye

Yes please run this command inside the folder that contains your Maester tests or as @DrPye suggested provide a path.

I think we should detect if there is a folder called maester-tests and prompt if the user wishes to update that folder. That way we can avoid this scenario if the folder already has tests.

merill avatar May 14 '24 13:05 merill

That worked. Thanks image

dafuni avatar May 14 '24 15:05 dafuni

Updating this to be implemented as an enhancement.

Update-MaesterTests should be updated to

  • Check if the current folder contains more folders than the four standard folders in our tests folder (ie CISA, Custom, EIDSCA, Maester).
    • If the four folder are missing or if there are more than four folders found it should error out with a message asking the user to run the command inside the maester-tests folder.

merill avatar May 15 '24 12:05 merill

@merill looking at the code we might have a simpler solution already.

If you look at the following snippet from Update-MaesterTests

    $MaesterTestsPath = Get-MtMaesterTestFolderPath

    if (-not (Test-Path -Path $MaesterTestsPath)) {
        Write-Error "Maester tests not found at $MaesterTestsPath"
        return
    }

    $targetFolderExists = (Test-Path -Path $Path)

    $installOrUpdate = if ($Install) { "installed" } else { "updated" }
    if ($targetFolderExists) {
        # Check if the folder already exists and prompt user to confirm overwrite.
        $itemsToDelete = Get-ChildItem -Path $Path -Exclude "Custom"

We could just update Get-MtMaesterTestFolderPath to accept a path param and default to psscriptroot if not provided.

We could then update test-path and get-childitem to use $MaesterTestPath instead of $path

The only downside with my method is that the folder will always have to be called maester-tests, which I personally think is a good thing for a framework.

If you like my suggestion ill get something together. (Sorry if I'm way off with this I'm brand new at open source contributing)

DrPye avatar May 15 '24 19:05 DrPye

@merill looking at the code we might have a simpler solution already.

We could just update Get-MtMaesterTestFolderPath to accept a path param and default to psscriptroot if not provided.

We could then update test-path and get-childitem to use $MaesterTestPath instead of $path

The only downside with my method is that the folder will always have to be called maester-tests, which I personally think is a good thing for a framework.

If the Path parameter defaulted to PSScriptRoot, would that end up pointing to the PowerShell module folder in {PSModulePath}\Maester\{Version}\maester-tests? If those tests were removed, the module would need to be reinstalled or updated. We would also need a different location to store custom tests in. Am I misunderstanding the concept? :)

SamErde avatar Jul 09 '24 10:07 SamErde

@merill looking at the code we might have a simpler solution already. We could just update Get-MtMaesterTestFolderPath to accept a path param and default to psscriptroot if not provided. We could then update test-path and get-childitem to use $MaesterTestPath instead of $path The only downside with my method is that the folder will always have to be called maester-tests, which I personally think is a good thing for a framework.

If the Path parameter defaulted to PSScriptRoot, would that end up pointing to the PowerShell module folder in {PSModulePath}\Maester\{Version}\maester-tests? If those tests were removed, the module would need to be reinstalled or updated. We would also need a different location to store custom tests in. Am I misunderstanding the concept? :)

No bud, your right. I've forgotten what I was thinking at the time but just had a quick look and I misunderstood what the Get-MtMaesterTestFolderPath was actually doing. I must've really thought I was onto something!

DrPye avatar Jul 09 '24 11:07 DrPye

I only noticed that because I tried to build the module locally last week and ran into a few errors that I had to figure out. Realizing the effect of using PSScriptRoot within a module was eye opening!

SamErde avatar Jul 09 '24 13:07 SamErde

@SamErde I've merged your PR for the updates. Would you be able to test the latest preview version and give a thumbs up?

https://www.powershellgallery.com/packages/Maester/0.2.9-preview

merill avatar Jul 11 '24 13:07 merill

Yes, I should be able to test in the next two days. Thanks!

SamErde avatar Jul 12 '24 02:07 SamErde

Tangential and low priority, but I chew on each time I run Update-MaesterTests, moving Remove-Item out of the loop should allow for that user interaction to be smoother. It may just be the recurse handling too, but haven't spent enough time thinking on it.

https://github.com/maester365/maester/blob/main/powershell/internal/Update-MtMaesterTests.ps1#L51

image

image

Snozzberries avatar Jul 14 '24 00:07 Snozzberries

@Snozzberries, very yes! I have been developing some thoughts about the update process and may start a conversation to preview and discuss the possibilities.

SamErde avatar Jul 15 '24 14:07 SamErde

@SamErde I've merged your PR for the updates. Would you be able to test the latest preview version and give a thumbs up?

https://www.powershellgallery.com/packages/Maester/0.2.9-preview

Looks good, but I did notice one minor issue: now that the Update-MaesterTests function includes validation of the folder and actually checks for Maester tests in the provided path -- if there are no tests found, it simply returns the message "Maester tests updated successfully!" instead of saying "No Maester tests were found in the provided path." I can add that as a follow-up.

image

SamErde avatar Jul 15 '24 14:07 SamErde

Thanks a lot @SamErde.

For the Update-MaesterTests on an empty folder, this is the expected behavior.

If the folder is empty it should simply install the files so they end up in a state where they have the latest tests.

merill avatar Aug 04 '24 21:08 merill