Add folder name to test weaver #1298
Conversation
GeertvanHorrik
requested review from
SimonCropp,
ltrzesniewski and
tom-englert
| @@ -12,13 +12,20 @@ public static TestResult ExecuteTestRun( | |||
| bool runPeVerify = true, | |||
| Action<ModuleDefinition>? afterExecuteCallback = null, | |||
| Action<ModuleDefinition>? beforeExecuteCallback = null, | |||
| string? folderName = null, | |||
There was a problem hiding this comment.
If you move this new parameter to the end, it should be fully backwards compatible
There was a problem hiding this comment.
I have though about it. You mean behind the assemblyName or really at the end? On the long term, I think it's better to keep them together, but short-term would be best to add at the very end.
There was a problem hiding this comment.
I agree the folderName parameter should be put at the very end, this avoids a breaking change.
There was a problem hiding this comment.
I had a very similar issue in InlineIL, and I had to basically rewrite WeaverTestHelper, so I completely understand the need for this change. 🙂
| @@ -12,13 +12,20 @@ public static TestResult ExecuteTestRun( | |||
| bool runPeVerify = true, | |||
| Action<ModuleDefinition>? afterExecuteCallback = null, | |||
| Action<ModuleDefinition>? beforeExecuteCallback = null, | |||
| string? folderName = null, | |||
There was a problem hiding this comment.
I agree the folderName parameter should be put at the very end, this avoids a breaking change.
|
Updated the PR based on feedback. Thanks! |
See #1297