Skip to content

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Jan 11, 2020

PR Summary

  • Remove Unicode BOM from code files (Remove UTF-8 BOM from text files #11546 was incomplete)
  • Insert final newline into code files, as per .vscode/settings.json
  • Clean up use of BeOfType parameter: remove unnecessary braces

PR Context

PR Checklist

@xtqqczze
Copy link
Contributor Author

@iSazonov PR is ready to merge, CodeFactor issues are not new.

@daxian-dbw
Copy link
Member

daxian-dbw commented Jan 13, 2020

Pass 'BeOfType' parameter as [RuntimeType] instead of [string]

I don't think changes related to this one are needed. -BeOfType takes a string as the type name, and this is how it's used in Pester's sample code.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Jan 13, 2020

@daxian-dbw Judging from BeOfType tests, it is intended for -BeOfType to take a RuntimeType as well as a string. Apparently this is a documentation issue for pester.

Passing a RuntimeType might be preferable because:

  • it could be recognised as a type in the AST
  • the type object is constructed in the same scope as the assertion, e.g.:
using namespace System.Management.Automation
Describe "scope test"  {
    $p = [ActionPreference]::Continue
    it "should recognise type passed explicitly" {
        $p | Should -BeOfType ([ActionPreference]) # Passes
    }
    it "should recognise type passed as string" {
        $p | Should -BeOfType ActionPreference     # Fails
    }
}

@iSazonov
Copy link
Collaborator

Usually we use $p | Should -BeOfType [ActionPreference]

@xtqqczze
Copy link
Contributor Author

@iSazonov The parentheses are necessary for the parser to treat the token [ActionPreference] as an expression. Otherwise it is parsed as a string anyway:

$p | Should -BeOfType [ActionPreference]   # Fails

@daxian-dbw
Copy link
Member

@xtqqczze I won't say either a RuntimeType or a string is preferable. Both are officially supported and both are useful. If you don't expect the type resolution to depend on anything from your script scope, then using a string is just fine. So I don't think it's necessary to change all -BeOfType instances to use a RuntimeType. We will not enforce this in our tests and gradually, it will again be a mix of both in our tests.

Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

LGTM with one comment.

@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Jan 15, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.1 milestone Jan 15, 2020
```powershell
$_ -ireplace '(\| Should(?: -Not)? -Be)\snull\b','$1 $null'
```
``powershell
$_ -ireplace '(\| Should(?: -Not)?)\s-?beoftype\s([a-z]\S*)','$1 -BeOfType ([$2])'
```
```powershell
$_ -ireplace '(\| Should(?: -Not)?)\s-?beoftype\s"([^"]+)"','$1 -BeOfType ([$2])'
```
```powershell
$_ -ireplace "(\| Should(?: -Not)?)\s-?beoftype\s'([^'']+)'",'$1 -BeOfType ([$2])'
```
@xtqqczze xtqqczze force-pushed the update-pester-syntax2 branch from 0c021ed to bd1ad61 Compare January 15, 2020 13:53
@xtqqczze
Copy link
Contributor Author

rebased to resolve merge confilcts

@daxian-dbw daxian-dbw merged commit 920b671 into PowerShell:master Jan 24, 2020
@ghost
Copy link

ghost commented Mar 26, 2020

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Test Indicates that a PR should be marked as a test change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants