TylerLeonhardt / vscode-pester-test-adapter

MIT License
33 stars 13 forks source link

Discovery should start from workspaceroot by Default #36

Open JustinGrote opened 3 years ago

JustinGrote commented 3 years ago

By having the extension search 'test','Tests',etc. it behaves differently than the normal Pester. For instance, if you have a Tests folder at the root and a Tests folder in a subdirectory (common for SecretManagement vault extensions), it only picks up the tests in the root Tests folder. I assumed I was doing something wrong so I spent a lot of time figuring out why this was happening until I found the actual execution code in the log.

Workaround

Set "pesterExplorer.testRootDirectory": ".", however I think this should be the default and it shouldn't try to "guess" the folders for you, it should be default discover what a standard Invoke-Pester in the root of the directory would find unless you specify otherwise with rootdirectory

Further testRootDirectory should be an array and run the invocation as a foreach across them and concatenate the results, but that's not as big of a deal.

TylerLeonhardt commented 3 years ago

I added this feature because of @PrzemyslawKlys. We came to the idea the most modules would just have a Tests folder and @Przemyslaw uses a non-Pester test file called HisModuleName.Tests.ps1 in the root of the repo to invoke the tests but Pester was running that file and it was all very confusing... hence the 46 comments of this issue:

https://github.com/TylerLeonhardt/vscode-pester-test-adapter/issues/16#issuecomment-749820624

I think the best compromise I can do here is check if there's a Tests folder anywhere else in the folder structure... and if there is, then use the rootdirectory. Otherwise, use "./Tests"

JustinGrote commented 3 years ago

I mean .Tests.ps1 is so de-facto Pester these days he really should have just named it to something else, I don't think I'd ever see a .tests.ps1 file and think it wasn't a Pester file :)

That's fine, the workaround accomplishes what I need so probably don't need to invest more in here but I'll leave it as an open issue till a final decision is made.

Thanks again @TylerLeonhardt, this extension has shortened my local development loop by at least 50%

PrzemyslawKlys commented 3 years ago

I have .Tests. And it is a pester file. Just the execution of all tests pester file 😜

TylerLeonhardt commented 3 years ago

I keep thinking about this more and the more I feel like the existing behavior is fine. There will be edge cases like building a secret management extension but for those, that's what the setting is there for.

I'm not sure how I could make it more discoverable that that setting exists without annoying the user...

but in this case there are pros and cons with both "leaving it at Root" and "being smart" but since the smarts is already implemented, I'm going to rule in favor of that lol.

If more and more people get burned by this issue though, I'm happy to change it back.

JustinGrote commented 3 years ago

Like I said, the workaround of just adding "." works fine, I'd say there just needs to be some clear documentation around that because it was not easy for me to figure out and I in general know what I'm doing, at least compared to a beginner who may be totally turned off that it's not finding his tests.