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

Include more statements in code coverage #2554

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fflaten
Copy link
Collaborator

@fflaten fflaten commented Aug 3, 2024

PR Summary

Extends code coverage to include break, continue, exit and throw statements.
return is not included due to inconsistent behavior (though return 123 is already covered as a command expression)

Adds sort after Group-Object to save future headaches with inconsistent report/output order in Windows PowerShell vs PowerShell 6.1 which sorts by default.

Fix #1465

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)

@fflaten
Copy link
Collaborator Author

fflaten commented Aug 3, 2024

Draft PR due to merge conflict with #2553 and #2298. Need to merge one, fix next + repeat due to hardcoded reports.

src/functions/Coverage.ps1 Show resolved Hide resolved
Comment on lines 293 to 301
$predicate = {
$args[0] -is [System.Management.Automation.Language.DynamicKeywordStatementAst] -or
$args[0] -is [System.Management.Automation.Language.CommandBaseAst]
$args[0] -is [System.Management.Automation.Language.CommandBaseAst] -or
$args[0] -is [System.Management.Automation.Language.BreakStatementAst] -or
$args[0] -is [System.Management.Automation.Language.ContinueStatementAst] -or
$args[0] -is [System.Management.Automation.Language.ExitStatementAst] -or
$args[0] -is [System.Management.Automation.Language.ThrowStatementAst]
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only updated PSv5+ predicate as we might remove the redundant predicate for v6.
A potential backport would need to extend both predicates.

@fflaten fflaten marked this pull request as ready for review October 31, 2024 17:42
@@ -110,6 +110,7 @@ function Write-NUnitTestSuiteElements {

$suites = @(
# Tests only have GroupId if parameterized. All other tests are put in group with '' value
# PowerShell 6.1+ sorts by default in Group-Object. We need to sort for consistent output in Windows PowerShell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This says we sort but the code below does not use sort? Is this a mistake?

Copy link
Collaborator Author

@fflaten fflaten Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very likely. Good catch, will fix. :)

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

Successfully merging this pull request may close these issues.

Code coverage not listing all missed commands
2 participants