FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Update format-hex tests to include -TestCase parameter by MiaRomero · Pull Request #3800 · PowerShell/PowerShell · GitHub

/ PowerShell Public

Update format-hex tests to include -TestCase parameter - #3800

Merged
Mike Richmond (mirichmo) merged 5 commits into
PowerShell:masterfrom
MiaRomero:formatHex-test-fix
Aug 16, 2017
ConversationCommits (5)ChecksFiles changed
Merged

Update format-hex tests to include -TestCase parameter#3800
Mike Richmond (mirichmo) merged 5 commits into
PowerShell:masterfrom
MiaRomero:formatHex-test-fix

Conversation

Maria Romero (MiaRomero) commented May 17, 2017

Copy link
Copy Markdown
Member

Addressing issue #3382 'Use -TestCases parameter in FormatHex.Tests.ps1'.

  • Replaced the 'foreach' with the -TestCases parameter in the test helper functions.

James Truher (JamesWTruher) reviewed May 18, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

James Truher (JamesWTruher) May 18, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think that these functions should just be inlined into the appropriate context block. Having these functions only makes things a little harder to find. it allows the test case data be closer to the actual test.

Jason Shirk (lzybkr) commented May 18, 2017

Copy link
Copy Markdown
Contributor

Maria Romero (@MiaRomero) - the assignee of a PR is the person responsible for merging to master - and only maintainers can do that merge. We assign issues to regular contributors, but not the PR.

Jason Shirk (lzybkr) removed their request for review May 18, 2017 19:46

Mike Richmond (mirichmo) commented May 23, 2017

Copy link
Copy Markdown
Member

James Truher (@JamesWTruher) or Francisco Gamino (@Francisco-Gamino) - Do you guys have any other concerns or comments?

Mike Richmond (mirichmo) commented May 25, 2017

Copy link
Copy Markdown
Member

Maria Romero (@MiaRomero) - We need to add you to the Microsoft organization. I just noticed that you are not a member yet.

Ilya (iSazonov) reviewed May 25, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We should check $output.Count >=1 or =1 not $null

Maria Romero (MiaRomero) Jun 6, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Changed.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can remove - next line make the check.

Maria Romero (MiaRomero) Jun 6, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, removed in all mentioned instances.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can remove - next line make the check.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can remove - next line make the check.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can remove - next line make the check.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can remove - next line make the check.

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Ilya (iSazonov) May 25, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can remove - next line make the check.

Maria Romero (MiaRomero) commented Jun 6, 2017

Copy link
Copy Markdown
Member Author

James Truher (@JamesWTruher) fixed the >1 issue. The way it was before was not actually testing the contents of the second file so when I corrected it, those tests failed because the output was on two lines. I changed the input text to avoid the hassle of having two lines of output in the second expected result (then had to create a file with longer text to test the buffer underrun).

Maria Romero (MiaRomero) mentioned this pull request Jun 15, 2017
Closed
James Truher (JamesWTruher) reviewed Jun 30, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

Context "Continues to Process Valid Paths" {

$skipTest = ([System.Management.Automation.Platform]::IsLinux -or [System.Management.Automation.Platform]::IsOSX)

James Truher (JamesWTruher) Jun 30, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd be happier if this was combined with the construct in 378
IsLinux -or IsOSX -or (-not $certProviderAvailable)
then 378 could just be:
It "<Name>" -Skip:$skip
same with the code below

Aditya Patwardhan (adityapatwardhan) requested changes Aug 16, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
}

Context "Path Paramater" {
Context "Path and LiteralPath Paramaters" {

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Typo in Paramaters -> Parameters

Maria Romero (MiaRomero) Aug 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
$output = Format-Hex -LiteralPath $InvalidPath, $inputFile1 -ErrorVariable errorThrown -ErrorAction SilentlyContinue
}

$errorThrown.FullyQualifiedErrorId | Should Match $ExpectedFullyQualifiedErrorId

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please use ShouldBeErrorId, like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Diagnostics/New-WinEvent.Tests.ps1

Line 27 in 0b7451b

{ New-WinEvent -ProviderName NonExistingProvider -Id 0 } | ShouldBeErrorId 'System.ArgumentException,Microsoft.PowerShell.Commands.NewWinEventCommand'

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
$thrownError = $_
}

$thrownError.FullyQualifiedErrorId | Should Match $ExpectedFullyQualifiedErrorId

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Use ShouldBeErrorId like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Diagnostics/New-WinEvent.Tests.ps1

Line 27 in 0b7451b

{ New-WinEvent -ProviderName NonExistingProvider -Id 0 } | ShouldBeErrorId 'System.ArgumentException,Microsoft.PowerShell.Commands.NewWinEventCommand'

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
}

$result | Should BeOfType 'Microsoft.PowerShell.Commands.ByteCollection'
$result[0].ToString() | Should Match $ExpectedResult

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

if ($result.count -gt 1)
{
$result[1].ToString() | Should Match $ExpectedSecondResult

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated

$result.count | Should Be $Count
$result | Should BeOfType 'Microsoft.PowerShell.Commands.ByteCollection'
$result[0].ToString() | Should Match $ExpectedResult

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
$errorThrown.FullyQualifiedErrorId | Should Match $ExpectedFullyQualifiedErrorId

$output.Length | Should Be 1
$output[0].ToString() | Should Match $inputText1

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
$result[1].ToString() | Should be "00000010 72 65 20 74 65 78 74 re text "
$result.Count | Should Be 3
$result[0].ToString() | Should be "00000000 4E 6F 77 20 69 73 20 74 68 65 20 77 69 6E 74 65 Now is the winte"
$result[1].ToString() | Should be "00000010 72 20 6F 66 20 6F 75 72 20 64 69 73 63 6F 6E 74 r of our discont"

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
$result[0].ToString() | Should be "00000000 54 68 69 73 20 69 73 20 61 20 62 69 74 20 6D 6F This is a bit mo"
$result[1].ToString() | Should be "00000010 72 65 20 74 65 78 74 re text "
$result.Count | Should Be 3
$result[0].ToString() | Should be "00000000 4E 6F 77 20 69 73 20 74 68 65 20 77 69 6E 74 65 Now is the winte"

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/FormatHex.Tests.ps1 Outdated
$result.Count | Should Be 3
$result[0].ToString() | Should be "00000000 4E 6F 77 20 69 73 20 74 68 65 20 77 69 6E 74 65 Now is the winte"
$result[1].ToString() | Should be "00000010 72 20 6F 66 20 6F 75 72 20 64 69 73 63 6F 6E 74 r of our discont"
$result[2].ToString() | Should be "00000020 65 6E 74 ent "

Aditya Patwardhan (adityapatwardhan) Aug 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should use MatchExactly like here:

PowerShell/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1

Line 499 in 7b8c6e7

$ci[1].Name | Should MatchExactly $filenamePattern

Maria Romero (MiaRomero) commented Aug 16, 2017

Copy link
Copy Markdown
Member Author

Thanks Aditya Patwardhan (@adityapatwardhan) for the comments - fixed all the Match to MatchExactly.

Aditya Patwardhan (adityapatwardhan) approved these changes Aug 16, 2017
View reviewed changes

Aditya Patwardhan (adityapatwardhan) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Mike Richmond (mirichmo) merged commit ea77b57 into PowerShell:master Aug 16, 2017
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
Update Format-Hex tests to include -TestCase parameter (PowerShell#3800)
c9944c8 
* update tests to include -TestCase paramater in It block

* Move test functions to appropriate context blocks

* combine skipTest conditions

* move skipTest to Describe block

* correct typo, change to MatchExactly
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Ilya (iSazonov) iSazonov left review comments
Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
Francisco Gamino (Francisco-Gamino) Awaiting requested review from Francisco-Gamino
Travis Plunk (TravisEz13) Awaiting requested review from TravisEz13
+1 more reviewer
James Truher (JamesWTruher) JamesWTruher left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

Mike Richmond (mirichmo)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL