8000 Added more tests for import-alias by file, regarding parsing 'difficult' aliases strings by SytzeAndr · Pull Request #9247 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Added more tests for import-alias by file, regarding parsing 'difficult' aliases strings#9247

Merged
adityapatwardhan merged 28 commits intoPowerShell:masterfrom
Geweldig:csv_import-alias_tests
Apr 11, 2019
Merged

Added more tests for import-alias by file, regarding parsing 'difficult' aliases strings#9247
adityapatwardhan merged 28 commits intoPowerShell:masterfrom
Geweldig:csv_import-alias_tests

Conversation

@SytzeAndr
Copy link
Contributor
@SytzeAndr SytzeAndr commented Mar 29, 2019

PR Summary

According to this comment by daxian-dbw, "abc""def"should be parsed to abc"def. This was not covered in the old test cases, but should be now with this PR.

This PR adds tests for the import-alias by file, in a seperate PR as discussed in #9091. After this gets accepted, it should be possible to start refactoring import-alias. Details about this refactorization can be found in #9091 .

This pr adds the following test cases for import alias by file:

  • "abc""def" should parse to abc"def
  • "aaa" should parse to aaa
  • "a,b" should parse to a,b
  • If an alias file is read with a line containing more than four values, throw an error
  • If an alias file is read with a line containing less than four values, throw an error

If more exhaustive testing is required, please let me know.

PR Checklist

@SytzeAndr SytzeAndr changed the title WIP: added more tests for import-alias, regarding parsing difficult aliases Added more tests for import-alias, regarding parsing difficult aliases Mar 29, 2019
@SytzeAndr SytzeAndr changed the title Added more tests for import-alias, regarding parsing difficult aliases Added more tests for import-alias, regarding parsing 'difficult' aliases strings Mar 29, 2019
@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Mar 29, 2019
@SytzeAndr SytzeAndr changed the title Added more tests for import-alias, regarding parsing 'difficult' aliases strings Added more tests for import-alias by file, regarding parsing 'difficult' aliases strings Mar 30, 2019
…ked by get-alias instead of executing the alias, which is a cleaner method to check whether the alias is imported correctly
@SytzeAndr
Copy link
Contributor Author
SytzeAndr commented Mar 31, 2019

We should now be able to safely delete pesteralias.txt in PowerShell\test\powershell\Modules\Microsoft.PowerShell.Utility\assets since the file to read is created in a temporary test folder and deleted afterwards. I think it is nice to remove it, any opinions on this?

@SytzeAndr
Copy link
Contributor Author
SytzeAndr commented Mar 31, 2019

I accidently commited the content of #9091 branch into this branch. I'll try to undo it

@SytzeAndr SytzeAndr requested review from JamesWTruher and PaulHigin as code owners

apparently this happened during the accidental merge due to code ownership. Although I fixed that the commits were reverted, I don't know if this request for review is a problem of any kind.

@SytzeAndr SytzeAndr force-pushed the csv_import-alias_tests branch from 0e7b95f to d2f1311 Compare March 31, 2019 20:13
@SytzeAndr
Copy link
Contributor Author
SytzeAndr commented Apr 2, 2019

All suggestions are implemented,thanks @iSazonov.

SteveL-MSFT and others added 3 commits April 3, 2019 11:48
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
SteveL-MSFT and others added 10 commits April 3, 2019 11:49
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
@SytzeAndr
Copy link
Contributor Author

@SteveL-MSFT I implemented all your suggestions

Copy link
Member
@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator
@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

@SytzeAndr Thanks for fixing all style issues in the old file!

adityapatwardhan and others added 2 commits April 10, 2019 18:42
…ias.Tests.ps1

Co-Authored-By: SytzeAndr <s.p.e.andringa@student.tudelft.nl>
@iSazonov
Copy link
Collaborator

@adityapatwardhan Could you please update your review?

@adityapatwardhan adityapatwardhan merged commit 4884317 into PowerShell:master Apr 11, 2019
@adityapatwardhan
Copy link
Member

@SytzeAndr Thank you for your contribution

@adityapatwardhan adityapatwardhan added this to the 7.0.0-preview.1 milestone Apr 11, 2019
@SytzeAndr SytzeAndr deleted the csv_import-alias_tests branch April 15, 2019 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Test Indicates that a PR should be marked as a test change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0