GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Update format-hex tests to include -TestCase parameter by MiaRomero · Pull Request #3800 · PowerShell/PowerShell

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label

May 17, 2017

Maria Romero (MiaRomero) requested review from Francisco Gamino (Francisco-Gamino), James Truher (JamesWTruher) and Jason Shirk (lzybkr)

May 17, 2017 22:12

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mirichmo reacted with thumbs up emoji

All reactions

👍 1 reaction

Mike Richmond (mirichmo) self-assigned this

May 18, 2017

Maria Romero (MiaRomero) assigned Maria Romero (MiaRomero) and unassigned Mike Richmond (mirichmo)

May 18, 2017

Jason Shirk (lzybkr) assigned Mike Richmond (mirichmo) and unassigned Maria Romero (MiaRomero)

May 18, 2017

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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.

TravisEz13 reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Maria Romero (MiaRomero) Jun 6, 2017

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Changed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

Ilya (iSazonov) May 25, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove - next line make the check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Maria Romero (MiaRomero) Jun 6, 2017

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks, removed in all mentioned instances.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

Ilya (iSazonov) May 25, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove - next line make the check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

Ilya (iSazonov) May 25, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove - next line make the check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

Ilya (iSazonov) May 25, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove - next line make the check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

Ilya (iSazonov) May 25, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove - next line make the check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

Ilya (iSazonov) May 25, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove - next line make the check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Maria Romero added 2 commits

June 6, 2017 11:56


          update tests to include -TestCase paramater in It block

323ddda


          Move test functions to appropriate context blocks

84a926b

Maria Romero (MiaRomero) force-pushed the formatHex-test-fix branch from 8ef74e2 to 84a926b Compare

June 6, 2017 19:06

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).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Maria Romero (MiaRomero) mentioned this pull request

Jun 15, 2017

Fixed test to not create a file called 1 when run #4021

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          combine skipTest conditions

8d89709

Maria Romero (MiaRomero) requested review from Travis Plunk (TravisEz13) and Aditya Patwardhan (adityapatwardhan) as code owners

August 16, 2017 17:35


          move skipTest to Describe block

d10847b

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Typo in Paramaters -> Parameters

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Maria Romero (MiaRomero) Aug 16, 2017

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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'

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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'

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          correct typo, change to MatchExactly

a0ca240

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) approved these changes

Aug 16, 2017

View reviewed changes

Aditya Patwardhan (adityapatwardhan) left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mike Richmond (mirichmo) merged commit ea77b57 into PowerShell:master

Aug 16, 2017

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/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

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Mike Richmond (mirichmo)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.