Skip to content

Fix typo in AMSI test#8561

Merged
iSazonov merged 4 commits intoPowerShell:masterfrom
iSazonov:fix-alias-intest
Dec 29, 2018
Merged

Fix typo in AMSI test#8561
iSazonov merged 4 commits intoPowerShell:masterfrom
iSazonov:fix-alias-intest

Conversation

@iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Dec 29, 2018

PR Summary

Now CI-Windows fail after #8546 : correct typo in cmdlet name in AMSI test.

PR Checklist

@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 29, 2018
@iSazonov iSazonov self-assigned this Dec 29, 2018
@iSazonov iSazonov requested a review from SteveL-MSFT December 29, 2018 05:35

$EICAR_STRING_B64 = "awZ8EmMWc3JjaAdvY2lrBgcbY20aBHBwGgROF3Z6cHJhHmBncn13cmF3HnJ9Z3plemFmYB5ndmBnHnV6f3YSF3sYexk= "
$bytes = [System.Convert]::FromBase64String($EICAR_STRING_B64)
$EICAR_STRING = -join ($bytes | ForeEach-Object { [char]($_ -bxor 0x33) })
Copy link
Collaborator

@vexx32 vexx32 Dec 29, 2018

Choose a reason for hiding this comment

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

The issue isn't the use of alias or not; the issue is that the cmdlet name was misspelled (ForeEach-Object should be ForEach-Object). I think removing the alias was the right move; we should keep the proper full cmdlet name.

Why did this not come up in the initial PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I'll try cmdlet name. In initial PR we did not run all tests with [Feature].

Copy link
Member

Choose a reason for hiding this comment

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

Seems like we should be able to detect that the change is within a Feature test and automatically run those, let me think about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CoreFX team uses a bot to run tests. We could do the same.
Perhaps it makes sense to run short set of tests initially for speed. Then always run full set of tests before merge.

Copy link
Member

Choose a reason for hiding this comment

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

@iSazonov that's a good idea. Maintainers can request bot to run full set of tests after review is approved and CI tests pass. Let me work on this.

@iSazonov iSazonov changed the title Revert change in AMSI test Fix typo in AMSI test Dec 29, 2018
@iSazonov
Copy link
Collaborator Author

@SteveL-MSFT Please review to fix night tests.

@iSazonov iSazonov merged commit f31750c into PowerShell:master Dec 29, 2018
@iSazonov iSazonov deleted the fix-alias-intest branch December 29, 2018 13:16
@iSazonov iSazonov restored the fix-alias-intest branch December 31, 2018 07:32
@iSazonov iSazonov deleted the fix-alias-intest branch December 31, 2018 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments