Test Results Files by Directory #407
Conversation
|
Thank you for your contribution! I will have time to look into it on Tuesday latest. |
There was a problem hiding this comment.
If we change the argument list, or the meaning of an argument, we should also update the documentation. The documentation can be found in the https://github.com/picklesdoc/docs repository.
| @@ -39,7 +39,23 @@ public void ReportOn(IConfiguration configuration, Action<string> writeToLog) | |||
| if (configuration.HasTestResults) | |||
| { | |||
| writeToLog($"Test Result Format : {configuration.TestResultsFormat}"); | |||
| writeToLog($"Test Result File : {configuration.TestResultsFile.FullName}"); | |||
| int result = 0; | |||
There was a problem hiding this comment.
You can write this entire block much simpler using some linq expressions.
| { | ||
| configuration.AddTestResultFiles( | ||
| this.fileSystem.DirectoryInfo.FromDirectoryName( | ||
| this.testResultsDirectory).EnumerateFiles("*.json", System.IO.SearchOption.AllDirectories)); |
There was a problem hiding this comment.
This restricts the results directory argument to .json result files.
I think it would be better to use the link-results-file parameter and make it able to read a wildcard expression.
For example, the link-results-file parameter should be able to interpret c:\temp\tests\*.trx.
| @@ -69,6 +69,7 @@ private static int Main(string[] args) | |||
| { | |||
| if (Log.IsFatalEnabled) | |||
| { | |||
| Log.Info(ex.StackTrace); | |||
| { | ||
| exampleResult = this.results.GetExampleResult(scenarioOutline, exampleValues); | ||
| } | ||
| catch (Exception) { } |
There was a problem hiding this comment.
You're swallowing an exception without handling it in any way. That's bad.
| List<Feature> features = this.ReadResultsFile(fileInfo); | ||
| if (features == null) | ||
| { | ||
| Console.WriteLine("There was an issue with the results format for " + fileInfo.Name); |
There was a problem hiding this comment.
You're writing to Console.WriteLine instead of using the Logging infrastructure.
| } | ||
| catch (Exception e) | ||
| { | ||
| Log.Error("An exception occured in aligning results with the feature for : " + |
There was a problem hiding this comment.
You're blindly logging all exceptions but not taking appropriate action. Not good.
| .Select(row => row.Result) | ||
| .Merge(); | ||
| } catch (Exception e) { | ||
| Log.Error("An exception occured in aligning results with the feature for : " + |
There was a problem hiding this comment.
Again: you're blindly logging an exception but taking no appropriate action. You should at the very least rethrow the exception. You should also be more selective in which exceptions you catch.
| @@ -16,6 +16,22 @@ | |||
| <NuGetPackageImportStamp> | |||
| </NuGetPackageImportStamp> | |||
| <TargetFrameworkProfile /> | |||
| <PublishUrl>C:\Users\aadams\Documents\ADP\published_files\pickles\</PublishUrl> | |||
There was a problem hiding this comment.
We don't need clickonce code in Pickles
| @@ -41,6 +57,18 @@ | |||
| <PropertyGroup> | |||
| <StartupObject>PicklesDoc.Pickles.CommandLine.Program</StartupObject> | |||
| </PropertyGroup> | |||
| <PropertyGroup> | |||
| <ManifestCertificateThumbprint>58C81F2E680EB86FABDBEE60F94068CFF0846BAC</ManifestCertificateThumbprint> | |||
There was a problem hiding this comment.
We also don't need this clickonce code in pickles
|
Thank you for continuing to spend time on this. I am close to merging PR #435 which adds wildcard support for test result files. I hope to release a new version of Pickles tomorrow, and request you try your scenario with the new version. I will not merge your pull request. I raised some questions in December (mainly about your code restricting the search to .json files) but you did not address those. Additionally, the error handling you added today is not up to scratch. I added some comments to individual lines. |
|
Yep, sorry about that. I actually forgot that this request was still open. I needed to add some quick and dirty error messaging to the project to essentially continue on error. I knew it wasn't ready to commit -- I do think you'd benefit from adding continue on error functionality though. The system as stands is pretty hard to use when dealing with large distributed teams and 100s if not 1000s of features. |
Changes to allow for passing test results files by directory instead of a delimited list of files. Code as it stands is designed for either the list of files or the directory. If both are passed, the directory trumps the list.