Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test discovery on linux fails to find tests in hidden folders #2515

Open
3 tasks done
tbutler-qontigo opened this issue Jun 28, 2024 · 7 comments
Open
3 tasks done

Test discovery on linux fails to find tests in hidden folders #2515

tbutler-qontigo opened this issue Jun 28, 2024 · 7 comments

Comments

@tbutler-qontigo
Copy link

Checklist

What is the issue?

I am writing tests for github actions which are in the .github folder so my tests are in tests/.github
Invoke-Pester does not find these tests unless I explicitly specify tests/.github

This works fine in windows powershell core but not in linux powershell core

I am using pester v5.6.0

Expected Behavior

Invoke-Pester finds all tests in all folders, including hidden folders, without having to explicitly specify the folder name
This should work in linux and in windows

Steps To Reproduce

  1. Place a tests in a folder named tests/.github
  2. Call Invoke-Pester from the parent folder

Observe that the test is discovered in windows but not in linux

Describe your environment

Working (in windows):

Pester version     : 5.6.0 C:\Program Files\WindowsPowerShell\Modules\Pester\5.6.0\Pester.psm1
PowerShell version : 5.1.22621.3672
OS version         : Microsoft Windows NT 10.0.22631.0

Not working (in WSL2 Ubuntu)

Pester version     : 5.6.0 /home/XXXX/.local/share/powershell/Modules/Pester/5.6.0/Pester.psm1
PowerShell version : 7.4.3
OS version         : Unix 5.15.153.1

Possible Solution?

No response

@fflaten
Copy link
Collaborator

fflaten commented Jun 30, 2024

Hi, thanks for the report.

Pester discover files using Get-ChildItem which by default excludes hidden files/folders.

You should see the same behavior on Windows by marking the folder as hidden, which is implicit for dot-prefixed items in Unix as you know.

There's usually a reason something is marked as hidden, so defaulting to including them feels wrong IMO.

May I ask why not just rename the dot-prefixed folder in tst to unhide it?

Related #2327

@tbutler-qontigo
Copy link
Author

Hi

You'd have to ask github why they use the .github name for their folders. - if they want them "hidden" for a reason or simply want them marked as "other" - ie not application code.

I am following the convention recommended here: https://pester.dev/docs/usage/file-placement-and-naming to place my tests " in a separate tests directory that follows the same directory structure as the main src directory."
Since the code being tested is in .github, the tests are in tests/.github

I would expect Pester to act consistently on different operating systems - if that means you always exclude folders that are considered "hidden" on one from the other operating system too then I would expect to have the ability to override that behaviour so that I can follow the recommded guidelines for test placement.

@fflaten
Copy link
Collaborator

fflaten commented Jul 1, 2024

You'd have to ask github why they use the .github name for their folders. - if they want them "hidden" for a reason or simply want them marked as "other" - ie not application code.

It's a common practice used to hide config, caches etc. from application code. Both to hide clutter and avoid accidental modifications because they're hidden on Unix.

I am following the convention recommended here: https://pester.dev/docs/usage/file-placement-and-naming to place my tests " in a separate tests directory that follows the same directory structure as the main src directory."

Since the code being tested is in .github, the tests are in tests/.github

Keep in mind that it's just a generic recommendation, which coincidentally was written while only Windows was supported.

Since you're working cross-platform I'd suggest renaming the folder to tests/github for now, at least until a change is implemented.

I'll update the page with a note about dot-prefixed files and folders being hidden on Unix. Will also have to update docs about the behavior of hidden files/folders.

I would expect Pester to act consistently on different operating systems - if that means you always exclude folders that are considered "hidden" on one from the other operating system too then I would expect to have the ability to override that behaviour so that I can follow the recommded guidelines for test placement.

Having a IncludeHidden setting is certainly an option. Be aware that it would also include your .git folder if your path is set to project root and any other more hidden content like caches causing overhead and slower performance.

IMO it's a lot easier to use explicit include paths for the very rare exceptions than having to maintain a potentially growing list of ExcludePath to reduce noise.

@tbutler-qontigo
Copy link
Author

Keep in mind that it's just a generic recommendation, which coincidentally was written while only Windows was supported.

This is a convention we follow anyway for all of our tests across all of our languages, unless there specifically isn't support for that.
Anyone spotting a folder discrepancy just for tests/github would probably think that it was a typo and "fix" it and not realise that tests would now not run.
We prefer not to have exceptions or have people rely on prior knowledge so they can concentrate on writing the code (and the tests) and not have to worry about edge cases.
It was a happy coincidence that the Pester recommendation was to follow a practice we already currently follow.

I think an IncludeHidden option would work where I specify just "tests" as the place to look for tests and it finds everything in every subfolder, so the .git folder would not be an issue in this case.
Doing this though you would need to ensure that windows also respects the ".folder is hidden" rule and linux should respect the windows way of marking a file as hidden so that you are consistent everywhere.

IMO it's a lot easier to use explicit include paths for the very rare exceptions than having to maintain a potentially growing list of ExcludePath to reduce noise.

For one of our repos as an example, our tests folder has immediate children of .github, scripts, templates - having to maintain an explicit list of ./tests/.github/**,./tests/scripts/**,./tests/templates/**" is painful and the next developer who adds tests in a different sub-folder of tests may not think to add it to the invocation in our CI pipeline.
Typically the code is written and tested locally in Windows so of course it all "just works" there.

Thanks

@fflaten
Copy link
Collaborator

fflaten commented Jul 1, 2024

Doing this though you would need to ensure that windows also respects the ".folder is hidden" rule and linux should respect the windows way of marking a file as hidden so that you are consistent everywhere.

I'm gonna have to disagree on this. Pester follows PowerShell's default behavior which adheres to the underlying OS and filesystem so we're consistent with other software on the same machine. Special-casing unix behavior in Windows will only cause confusion and unix can't read NTFS's Hidden attribute at all. Pester is already consistent by excluding hidden items on every platform.

Filesystem differences like how hidden is defined is just one of those things users need to be aware of when writing cross-platform PowerShell-code. Just like paths should use relative paths or environment variables for known locations, and forward-slash as directory separator to support both Windows and Unix filesystems.

having to maintain an explicit list of ./tests/.github/,./tests/scripts/,./tests/templates/**" is painful and the next developer who adds tests in a different sub-folder of tests may not think to add it to the invocation in our CI pipeline.

Using "/tests", "/tests/.github" should be enough. You only need to specify the hidden folder, the rest will be found using recursion unless you want to include only a subset of folders under tests.

Anyone spotting a folder discrepancy just for tests/github would probably think that it was a typo and "fix" it and not realise that tests would now not run.

tests/github-do-not-rename? 😇

Having hidden folders are very rare for code meant to be found, so this is likely the only scenario you'll need it. Workaround is simple and explictly including the rare static exceptions is easier than excluding other potentially generated items .git, .cache etc.

Personally I'd like to avoid configuration bloat, at least until we get more requests for this. Thoughts @nohwnd ?

@tbutler-qontigo
Copy link
Author

Fair enough about your opinion on consistency - everyone has their own opinion - I favour consistency for the developer, particularly when it becomes a "silent failure" - you have a bunch of tests, some in a .folder and some not. You see tests running in windows; you see tests running in linux - how many people are going to check that the counts match?

Using "/tests", "/tests/.github" should be enough. You only need to specify the hidden folder, the rest will be found using recursion unless you want to include only a subset of folders under tests.

I tried this initially but the .github tests ran twice on windows which is also not ideal.

Having hidden folders are very rare for code meant to be found

Except if you are writing github actions and reusable workflows, in which case it becomes very common because of the .github folder.
Potentially it is a bigger issue than just tests - what happens to code coverage? Even if we decided it was acceptable to have a "nonstandard" name for tests/github not mirroring .github, would coverage be calculated correctly because the .github folder is still classed as a hidden folder?

@nohwnd
Copy link
Member

nohwnd commented Jul 3, 2024

I am going to side with OP here. I think it is reasonable to either include hidden folders by default, with exclusion to .git.

I am not sure how to best expose this change to users, to me this is a "bug" on our side, so I would not object to fixing it in v5 without special flag to opt out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants