Shorten TestDrive directory name #2239

splatteredbits · 2022-08-11T18:24:48Z

PR Summary

Pester currently uses a full GUID as its temp directory name, which uses 36 characters. This contributes to test errors when paths used in tests exceed the Windows max path of 260 characters.

The [IO.Path]::GetTempFileName() only uses 4 characters for the unique/random part of its filename. Pester should follow the same pattern and only use a temp directory with 4 characters.

Let's not use a GUID anymore, either. A GUID, when converted to a string, only uses hex characters: 0-9 and a-f, which gives us 65,536 unique possible values. Instead, lets use the first four characters of the file name returned by [IO.Path]::GetRandomFileName(). It generates a name that uses all letters of the alphabet and the numbers 0-9, which gives us 36^4 = 1,679,616 possible values.

Fix #2238

PR Checklist

PR has meaningful title
Summary describes changes
PR is ready to be merged
- If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
Tests are added/update (if required)
Documentation is updated/added (if required)

Pester currently uses a full GUID as its temp directory name, which uses 36 characters. This contributes to test errors when paths used in tests exceed the Windows max path of 260 characters. The `[IO.Path]::GetTempFileName()` only uses 4 characters for the unique/random part of its filename. Follow the same pattern in `New-RandomTempDirectory` by only using the 4 characters. A GUID, when converted to a string, only uses hex characters: 0-9 and a-f, which gives us 65,536 unique possible values. Instead, lets use `[IO.Path]::GetRandomFileName()`. It generates a name that uses all letters of the alphabet and the numbers 0-9, which gives us 36^4 = 1,679,616 possible values.

pnstickne · 2022-08-11T20:55:33Z

src/functions/TestDrive.ps1

@@ -71,7 +71,8 @@ function Clear-TestDrive ([String[]]$Exclude, [string]$TestDrivePath) {
 function New-RandomTempDirectory {
    do {
        $tempPath = Get-TempDirectory
-        $Path = [IO.Path]::Combine($tempPath, ([Guid]::NewGuid()));
+        $dirName = [IO.Path]::GetRandomFileName().Substring(0, 4)


Perhaps 6 or 8.

I chose 4 because that's what the System.IO.Path.GetTempFileName() static method uses. It returns a filename with four unique hex characters. But I'm not against six or eight characters, either.

Which implementation of GetTempFileName() (or GetRandomFileName ?) are you looking at? .NET does not do it. And .NET Framework does that neither, by looking at the code, they both use cryptographically safe randomization and a file check.

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs#L277-L289

That said, running this on my computer produces 1-10 conflicts. Which we would resolve by file check, so it is not likely to cause too many problems.

1..1000000 | % { [IO.Path]::GetRandomFileName().Substring(0, 4) } | unique | Measure-Object

I was looking at [System.IO.Path]::GetTempFileName() as the basis for "four unique characters is enough", and not at how it's implemented under the covers. The problem with [IO.Path]::GetTempFileName() is that it actually creates a temporary file, so I don't want to do that.

nohwnd · 2022-10-02T11:04:47Z

Added good first issue to this. It should not be complex to replace the way the name is generated with the shortened ID. But what needs to be kept in place is the check that the item does not already exist.

Not sure why I thought this is an ask for feature, rather than PR.

nohwnd · 2022-10-02T11:06:22Z

@fflaten any opinion on shortening the names?

Maybe we should also unify this with Test Registry?

fflaten · 2022-10-02T11:17:22Z

@fflaten any opinion on shortening the names?

Sounds good, but 4 chars sounds a bit low without a Pester-prefix

Maybe we should also unify this with Test Registry?

Yes

splatteredbits · 2022-11-09T22:56:46Z

@fflaten any opinion on shortening the names?

Sounds good, but 4 chars sounds a bit low without a Pester-prefix

The current test directory name doesn't have a Pester-prefix, it uses an inscrutable Guid. The object is to use as short a name as possible to avoid path too long issues. Adding a prefix would partially defeat that.

nohwnd · 2022-11-10T06:26:53Z

@fflaten ~~I think we put them in our folder Temp/Pester/ ?~~ And same in registry? HKCU/../Pester/ and we double check that what we are creating does not exist yet, so imho it should be all okay.

We should just make it the same for registry, if that did not happen yet?

Update: I thought wrong, no Pester folder in TEMP. But we do doublecheck for conflicts though, so still okay?

fflaten · 2022-11-10T08:09:39Z

The current test directory name doesn't have a Pester-prefix, it uses an inscrutable Guid.

Currently if cleanup doesn't run as expected (ex. cancelled script), then those guid-folders will be unidentifiable. "What keeps filling up my temp and is it safe to delete?" By adding a source hint like pester somewhere in the path we'd tackle that while also reducing risk of conflict with as few chars as 4. But it's not a critical issue and was just a suggestion as we rarely touch this.

The object is to use as short a name as possible to avoid path too long issues. Adding a prefix would partially defeat that.

10 vs 36 is still a solid improvement. If the last 6 chars breaks your solution then you're one long username from breaking and should modify the test code itself. So this PR is mostly a quality improvement and not a "every char counts" critical fix imo. 🙂

src/functions/TestRegistry.ps1

splatteredbits mentioned this pull request Aug 11, 2022

Shorten length of temp directory name #2238

Closed

3 tasks

splatteredbits force-pushed the feature/2238-shorten-testdrive-directory-name branch from db52706 to fb84215 Compare August 11, 2022 18:35

splatteredbits marked this pull request as ready for review August 11, 2022 20:45

pnstickne approved these changes Aug 11, 2022

View reviewed changes

nohwnd added good first issue Help Wanted labels Oct 2, 2022

nohwnd removed good first issue Help Wanted labels Oct 2, 2022

fflaten changed the title ~~2238 Shorten TestDrive directory name.~~ Shorten TestDrive directory name Mar 23, 2023

fflaten added the Feature label Mar 30, 2023

nohwnd added this to the 6.0.0 milestone Apr 10, 2024

nohwnd added 3 commits May 17, 2024 22:09

Merge branch 'main' into feature/2238-shorten-testdrive-directory-name

251ce9e

shorter name in registry

515d9cd

Pester_ prefix

aa4d8c7

fflaten approved these changes May 18, 2024

View reviewed changes

src/functions/TestRegistry.ps1 Show resolved Hide resolved

nohwnd mentioned this pull request May 18, 2024

Update how test registry entry is generated #2459

Open

nohwnd merged commit 87cfcea into pester:main May 18, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shorten TestDrive directory name #2239

Shorten TestDrive directory name #2239

splatteredbits commented Aug 11, 2022 •

edited by fflaten

pnstickne Aug 11, 2022

splatteredbits Aug 11, 2022

nohwnd Aug 12, 2022 •

edited

nohwnd Aug 12, 2022

splatteredbits Aug 25, 2022 •

edited

nohwnd commented Oct 2, 2022 •

edited

nohwnd commented Oct 2, 2022

fflaten commented Oct 2, 2022

splatteredbits commented Nov 9, 2022

nohwnd commented Nov 10, 2022 •

edited

fflaten commented Nov 10, 2022 •

edited

Shorten TestDrive directory name #2239

Shorten TestDrive directory name #2239

Conversation

splatteredbits commented Aug 11, 2022 • edited by fflaten

PR Summary

PR Checklist

pnstickne Aug 11, 2022

Choose a reason for hiding this comment

splatteredbits Aug 11, 2022

Choose a reason for hiding this comment

nohwnd Aug 12, 2022 • edited

Choose a reason for hiding this comment

nohwnd Aug 12, 2022

Choose a reason for hiding this comment

splatteredbits Aug 25, 2022 • edited

Choose a reason for hiding this comment

nohwnd commented Oct 2, 2022 • edited

nohwnd commented Oct 2, 2022

fflaten commented Oct 2, 2022

splatteredbits commented Nov 9, 2022

nohwnd commented Nov 10, 2022 • edited

fflaten commented Nov 10, 2022 • edited

splatteredbits commented Aug 11, 2022 •

edited by fflaten

nohwnd Aug 12, 2022 •

edited

splatteredbits Aug 25, 2022 •

edited

nohwnd commented Oct 2, 2022 •

edited

nohwnd commented Nov 10, 2022 •

edited

fflaten commented Nov 10, 2022 •

edited