-
Notifications
You must be signed in to change notification settings - Fork 164
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.
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
Add wildcard support for test result files #435
Conversation
…mandLine, PowerShell, UI-api). Folders and no match will deliver no result file. Multiple matches on the same file will distinct into a single result.
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.
Cool work. If you can address my comments below soon, I will release a new version of Pickles this week with your contribution.
@@ -465,6 +571,7 @@ public void ThenCanParseResultsFormatSpecrunWithShortFormSuccessfully() | |||
[Test] | |||
public void ThenCanFilterOutNonExistingTestResultFiles() | |||
{ | |||
FileSystem.AddFile(@"c:\results1.xml", "<xml />"); |
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.
Why do we need this line?
|
||
Check.That(shouldContinue).IsTrue(); | ||
Check.That(configuration.HasTestResults).IsTrue(); | ||
Check.That(configuration.TestResultsFiles.Count()).IsEqualTo(2); |
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 for the unit tests. I like your test cases.
Here's a way to write the assertions with more focus on their intention:
Check.That(configuration.TestResultsFiles.Select(trf => trf.FullName)).ContainsExactly(@"c:\results1.xml", @"c:\results2.xml");
Please change that here and in the other test cases as well.
By the way: would you be willing to also adapt the documentation? You can find it at https://github.com/picklesdoc/docs/blob/master/docs/ArgumentsTestResultsFile.md |
PR for docs here: picklesdoc/docs#16 |
Thanks for the changes, they look good. One question still stands: why do we need |
Pickles throws an exception when a folder does not exist for a resultfile. The about selecting a non existing file was a victim of this exception making it fail. By ensuring the directory exists, the testcase passes.
@dirkrombauts You asked "One question still stands: why do we need FileSystem.AddFile(@"c:\results1.xml", ""); at line 574 of src/Pickles/Pickles.Test/WhenParsingCommandLineArguments.cs?", My previous answer got lost, hope this clarifies it: For wildcard support, we split up the directory path and the filename that could be a pattern. Therefore pickles throws an error if a path does not exist. No error is thrown if the file does not exist. In this particular testcase, it is tested that no error is thrown when a testresults file does not exist, that is why we need the directory to exist where the non existing file is located (that sounds like Schrödingers file haha). We have to make a choice: do we ignore it when a folder does not exist and just go on, or do we throw an exception on a not existing folder path. I prefer the latter since silent continue (aka Pokémon Exception Handling) is the root of all evil bugs. |
Thanks for your contribution! |
* Feature/add ignore tag (#433) * add .vs to gitignore * add IgnoreTag Feature * add ignoreTag parameter in any Runner * Fix after test with real case * Implement requests * Fix whitespace * Add properties and handling to model and viewmodel * Insert a row and move all rows down. * Add label and textbox for exclude tags * Reorganize UI * Rename Test Explude Tag "ignore-tag" to "exclude-tag" and add test for no sensitivity * Second try of wildcard support for testresult files (#435) * Added semicolon and wildcard support for all call types (MsBuild, CommandLine, PowerShell, UI-api). Folders and no match will deliver no result file. Multiple matches on the same file will distinct into a single result. * Improved design of tests for TestResultFiles in order to reflect their intention * Changed add file into add directory to the filesystem mock. Pickles throws an exception when a folder does not exist for a resultfile. The about selecting a non existing file was a victim of this exception making it fail. By ensuring the directory exists, the testcase passes. * Release 2.14.0 (#437) * Adapt change log * Version Bump (2.14.0) * Enable creation of outputs in deploy script * Version 2.14.0
Released in version 2.14.0. |
Hi,
I messed up with my fork, so I recreated a branch from scratch. This is the new Pull request for the topic discussed in this closed pull request.