Export-Csv Test Improvements #5150
Conversation
| @@ -1,73 +1,71 @@ | |||
| Describe "Export-Csv" -Tags "CI" { | |||
| $testObject = @("test","object","array") | |||
| $testCsv = Join-Path -Path $TestDrive -ChildPath "output.csv" | |||
There was a problem hiding this comment.
Please put the two lines in BeforeAll block.
|
|
||
| It "Should be able to be called without error" { | ||
| { $testObject | Export-Csv $testCsv } | Should Not Throw | ||
| { $testObject | Export-Csv $testCsv } | Should Not Throw |
There was a problem hiding this comment.
Please add -ErrorAction Stop.
Export-Csv $testCsv -ErrorAction Stop|
|
||
| It "Should throw if an output file isn't specified" { | ||
| { $testObject | Export-Csv -ErrorAction SilentlyContinue } | Should Throw | ||
| { $testObject | Export-Csv -ErrorAction SilentlyContinue } | ShouldBeErrorId "CannotSpecifyPathAndLiteralPath,Microsoft.PowerShell.Commands.ExportCsvCommand" |
There was a problem hiding this comment.
Please use -ErrorAction Stop.
| $piped = Get-Content $testCsv | ||
|
|
||
| $piped[0] | Should Match ".String" | ||
| $piped[0] | Should be "#TYPE System.String" |
There was a problem hiding this comment.
Please use:
$piped[0] | Should BeExactly "#TYPE System.String" | $switch = Get-Content $testCsv | ||
|
|
||
| $switch[0] | Should Match ".Object" | ||
| $switch[0] | Should Match ".Object" |
There was a problem hiding this comment.
Please use something like:
$piped[0] | Should BeExactly "#TYPE System.Object" There was a problem hiding this comment.
Done. But "#TYPE System.Object[]"
| It "Should be able to use the epcsv alias without error" { | ||
| { $testObject | Export-Csv -Path $testCsv } | Should Not Throw | ||
| { $testObject | epcsv -Path $testCsv } | Should Not Throw | ||
| } |
There was a problem hiding this comment.
Please remove the test at all - we have another file for alias tests.
| { $testObject | epcsv -Path $testCsv } | Should Not Throw | ||
| } | ||
|
|
||
| It "Should have the same information when using the alias vs the cmdlet" { |
There was a problem hiding this comment.
Please remove the test at all - we have another file for alias tests.
| } | ||
| } | ||
|
|
||
| It "Should have the same information when using the alias vs the cmdlet" { |
There was a problem hiding this comment.
Please remove the alias test.
|
|
||
| It "Should be able to use the epcsv alias without error" { | ||
| { $testObject | Export-Csv -Path $testCsv } | Should Not Throw | ||
| for ( $i = 0; $i -lt $testCsv.Length; $i++) { |
There was a problem hiding this comment.
I wonder why we use $testCsv.Length?
I'd expect $expected.Count.
Your thoughts?
There was a problem hiding this comment.
I agree and think it should be $expected.Count. I have no clue how the length of the temporary file path string is relevant to anything in this test.
There was a problem hiding this comment.
Is there even a need for looping logic here? would calling get-content once and perform 4 explicit tests be better? It seems like creating an array and looping through it (with a for loop no less) is just overkill.
There was a problem hiding this comment.
I think we should stop on $expected.Count
| # Clean up after yourself | ||
| Remove-Item $aliasObject -Force | ||
| for ( $i = 0; $i -lt $expected.Count; $i++) { | ||
| $(Get-Content $testCsv)[$i] | Should Be $expected[$i] |
There was a problem hiding this comment.
Please correct indentation.
|
@Marusyk Please resolve merge conflicts. |
|
@iSazonov Done |
|
appveyor build fail due to #5201 |
|
#5118 has been merged, and I restarted AppVeyor CI for this PR. |
|
@daxian-dbw Can we merge the PR? |
|
@iSazonov Sorry that I didn't get a chance to review this change (too many notifications since having the code owner filter ...). Please proceed as you see appropriate :) |
|
@Marusyk Thanks for your contribution! Welcome back with new PRs. |
iSazonov
added
the
Hacktoberfest
4 participants
Fix #5139