Skip to content

Change Get-FileHash tests to use raw bytes #6430

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.

Sign up for GitHub

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

Merged
adityapatwardhan merged 3 commits into from
Mar 20, 2018
Merged

Change Get-FileHash tests to use raw bytes #6430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3032477
[feature] Change filehash tests to use raw bytes
rjmholt Mar 19, 2018
3d2bedd
Remove testablescript.ps1 from Get-FileHash tests
rjmholt Mar 19, 2018
dd1b490
Use BOM-less UTF8 input for Get-FileHash
rjmholt Mar 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/Get-FileHash.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,23 @@
Describe "Get-FileHash" -Tags "CI" {

BeforeAll {
$testDocument = Join-Path -Path $PSScriptRoot -ChildPath assets testablescript.ps1
Copy link
Member

@SteveL-MSFT SteveL-MSFT Mar 19, 2018

Choose a reason for hiding this comment

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

I don't see that script being used anywhere else, so you should delete that script if it's no longer needed.

Copy link
Collaborator Author

@rjmholt rjmholt Mar 19, 2018

Choose a reason for hiding this comment

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

Deleted in latest commit

Write-Host $testDocument
$testDocument = Join-Path -Path $TestDrive -ChildPath "hashable.txt"
$utf8NoBOM = [System.Text.UTF8Encoding]::new($false)
[System.IO.File]::WriteAllText($testDocument, "Get-Module`n", $utf8NoBOM)
Copy link
Collaborator

@iSazonov iSazonov Mar 20, 2018

Choose a reason for hiding this comment

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

From the PR description:

a static asset, whose encoding depends on platform configurations

I wonder, please clarify - how encoding of the static file is changed dynamically depending on a platform?

Also I don't understand why we exclude BOM - it seems the cmdlet should works correctly with BOM and without BOM on all platforms.

Why we use the WriteAllText overload - last parameter is "append"?

Copy link
Collaborator Author

@rjmholt rjmholt Mar 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Excellent questions!

The default Git configuration on Windows machines means that LF is checked out as CRLF -- which added a CR byte to testablescript.ps1 and broke the test. Also, opening testablescript.ps1 in text editors caused the BOM to get deleted in some cases.

I would have been happy with leaving the BOM, or even just writing some arbitrary bytes to a file, but we decided that if .NET Core is trying to get away from using BOMs in UTF-8, we should encourage that everywhere. Also, it makes the tests easier to verify against other hashsum calculators in BOM-less UTF-8 environments, which are arguably more prevalent and more likely to have an independent implementation from ours.

Finally, the WriteAllText we use is [System.IO.File]::WriteAllText(String path, String text, Encoding fileEncoding), rather than [Microsoft.VisualBasic.FileIO.FileSystem]::WriteAllText(String file, String text, Boolean append). Which I also got caught up by a couple of times.

Copy link
Member

@SteveL-MSFT SteveL-MSFT Mar 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The hash should be based on the bytes of the file and the file encoding including BOM is opaque to the cmdlet. We have use -TestCases to have both BOM and no-BOM tests if needed.

}

Context "Validate platform encoding" {
# If this test fails, then the problem lies outside the Get-FileHash implementation
It "Should have the bytes in the file as expected" {
[System.IO.File]::ReadAllBytes($testDocument) | Should -BeExactly @(0x47, 0x65, 0x74, 0x2d, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x0a)
}
}

Context "Default result tests" {
It "Should default to correct algorithm, hash and path" {
$result = Get-FileHash $testDocument
$result.Algorithm | Should Be "SHA256"
$result.Hash | Should Be "8129a08e5d748ffb9361375677785f96545a1a37619a27608efd76a870787a7a"
$result.Hash | Should Be "41620f6c9f3531722efe90aed9abbc1d1b31788aa9141982030d3dde199f770c"
$result.Path | Should Be $testDocument
}
}
Expand All @@ -20,11 +28,11 @@ Describe "Get-FileHash" -Tags "CI" {
BeforeAll {
# Keep "sHA1" below! It is for testing that the cmdlet accept a hash algorithm name in any case!
$testcases =
@{ algorithm = "sHA1"; hash = "f262f3d36c279883e81218510c06dc205ef24c9b" },
@{ algorithm = "SHA256"; hash = "8129a08e5d748ffb9361375677785f96545a1a37619a27608efd76a870787a7a" },
@{ algorithm = "SHA384"; hash = "77cdffd27d3dcd5810c3d32b4eca656f3ce61cb0081c5ca9bf21be856c0007f9fef2f588bae512a6ecf8dc56618aedc3" },
@{ algorithm = "SHA512"; hash = "82e3bf7da14b6872b82d67af6580d25123b3612ba2dfcd0746036f609c7752e74af41e97130fbe943ec7b8c61549578176bff522d93dfb2f4b681de9f841c231" },
@{ algorithm = "MD5"; hash = "2d70c2c2cf8ae23a1a86e64ffce2bbca" }
@{ algorithm = "sHA1"; hash = "0c483659b1f2d5a8f116211de8f58bf45893cffb" },
@{ algorithm = "SHA256"; hash = "41620f6c9f3531722efe90aed9abbc1d1b31788aa9141982030d3dde199f770c" },
@{ algorithm = "SHA384"; hash = "ec4c4d4f0b2a79f216118c5a5059b8ce061097ba9161be5890c098aaeb5db169c13dae0a6f855c9a589cd11df47d0c87" },
@{ algorithm = "SHA512"; hash = "6aba8ba8b619100a6829beb940d9d77e4a8197fdcac2d0fe5ad6c2758dacc5a59774195fd8a7a92780b7582a966b81ca0c1576c0044c5af7be20f5ccf425bd76" },
@{ algorithm = "MD5"; hash = "f9d78bd059ab162bea21eb02badde001" }
}
It "Should be able to get the correct hash from <algorithm> algorithm" -TestCases $testCases {
param($algorithm, $hash)
Expand Down
1 change: 0 additions & 1 deletion test/powershell/Modules/Microsoft.PowerShell.Utility/assets/testablescript.ps1
View file
Open in desktop

This file was deleted.