-
Notifications
You must be signed in to change notification settings - Fork 325
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
Generation of CodeCoverage.deps.json file #2627
Generation of CodeCoverage.deps.json file #2627
Conversation
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.
Looks good to me, just small comments.
@@ -67,6 +67,14 @@ public void GetVanguardProfilerConfigX64PathShouldReturnRightDirectory() | |||
Assert.AreEqual(Path.Join(this.GetCurrentAssemblyLocation(), @"CodeCoverage\amd64\VanguardInstrumentationProfiler_x64.config"), actualPath); | |||
} | |||
|
|||
[TestMethod] | |||
public void GetCodeCoverageShimPathShouldReturnRightDirectory() |
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.
Prefer to rename to GetCodeCoverageShimPathShouldReturnCorrectDirectory
/// <summary> | ||
/// Name of element for custom coverage filename. | ||
/// </summary> | ||
private const string CoverageFileSettingName = "CoverageFileName"; | ||
|
||
private const string CodeCoverageDepsJsonTemplate = @" | ||
{{ |
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.
Will this work in different platforms? Is it better to concatenate strings with Environment.NewLine
?
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.
logic is invoked only on windows now, new lines doesn't matter for JSON files
|
||
var obj = JObject.Parse(File.ReadAllText(this.collectorImpl.CodeCoverageDepsJsonFilePath)); | ||
|
||
var expected = @" |
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.
The same as above.
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.
Since it's a json file, a new line shouldn't affect anything other than formatting.
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.
also this stuff is working only on Windows now
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.
LGTM
Description
Added logic to create by TraceDataCollector CodeCoverage.deps.json. This file is used to add reference to Shim dll to any child process from main test project (if verifiable probes are enabled).
Related issue
#1263