-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
🙏 Feature Request: AST-Based Test Discovery that doesn't run test discovery logic #13
Comments
The "ignore" thing is a known 5.1 bug, switch to 7 and it works fine. I haven't ironed out all the 5.1 compatability stuff. Also it only executes discovery logic in discovery. If you put things like your "Required modules" output above in a BeforeEach or BeforeAll it won't execute. |
I will track ignore in a separate issue, however the "Execution on expand" is required in order to discover tests and testcases. A no-execute AST-based method may be possible but I'm not gonna make it, I may consider PRs as a testProvider :) |
Well - the version of Tyler doesn't execute them - so there's that. Surely this is rather unexpected to get something executed while opening as it can have deadly consequences. |
@PrzemyslawKlys without it, there's no way to identify test cases without basically duplicating extensive AST parsing. Performing that action has little difference to the Invoke-Pester command with "NoRun" enabled, so if the "execution on discovery" is a concern, this is an issue that should be filed in the Pester repository and this repo will inherit that enhancement. I get your point in terms of "oh someone puts rm -rf * in the discovery logic" but that's a risk regardless. Tyler's method is here: https://github.com/TylerLeonhardt/vscode-pester-test-adapter/blob/master/src/powershellScripts.ts I'll reopen this issue and mark it as up-for-grabs. |
Well have you noticed other errors that happened? While one thing is auto-execution - the other is - it crashed, and have not detected a single thing :) |
@PrzemyslawKlys not quite sure what you mean but it's an 0.0.1 release, I expect there to be many bugs to squash at the moment :). You are also totally free to use Tyler's extension still, there is a new setting in test adapter that "ports" them to the new testing API |
@PrzemyslawKlys because I was curious about this, I checked and Tyler's extension actually does run discovery and execute code, you just don't see it because he runs it as a background runspace whereas I run it directly in the terminal. I plan to add background runspace support, but FYI. Here's the function Tyler calls (Find-Test): Which in turn calls discover-test which runs the discovery process: TO verify this write to a file on your desktop in your discovery logic. When I ran it, it showed up. |
I mean if you look at gif I presented - you will see that I expand the tests - and when I expand tests it starts to execute and starts throwing all sort of PowerShell errors which are not there when running manually. I understand it's 0.0.1 - I don't even use pester that much - just wanted to help report errors. I don't use Tyler's extension either because I'm simply not writing any tests lately :) |
Appreciated! as far as the error it's because you're running in 5.1 and not 7. Issue for that here, should be fixed next release: |
I'm going to close this issue because it makes more sense for Pester to implement this with a function or configuration option, and if it does, then this adapter can support it, I recommend opening this in the Pester repository if it is a desired feature. |
@PrzemyslawKlys to follow up on this, the Trusted Workspaces feature already prevents the extension from loading until a workspace is trusted. I may implement support to do the file-based pester discovery but not actual test discovery until a workspace is trusted, that is tracked here: |
It's related probably to: TylerLeonhardt/vscode-pester-test-adapter#16
And
The text was updated successfully, but these errors were encountered: