Skip to content

Commit b85d9b7

Browse files
committed
Add multiple safeties around the RemoveDir task
So that we don't end up nuking the whole git repository! It was my misunderstanding that targets setup with `AfterTargets` would not run if the target specified was skipped because its condition would evaluate to false. When trying to add multitargeting, the `MoveCoverletReport` target ran anyway and the `CoverletReport` item evaluated to empty which in turn made `CoverletReportParentDirectory` evaluate to `..`, i.e the root of the git repository which was then deleted by the `RemoveDir` task. Two safeties have been added: 1. Add the same `Condition="$(TargetFramework) != ''"` on all targets running after `GenerateHtmlCoverageReport`. 1. Add `Condition="$(CoverletReport) != ''"` when defining the `CoverletReportParentDirectory` item. This incident would never have happened in the first place if: 1. I could have used `$([System.IO.Directory]::GetParent($(CoverletReport)))` instead of `$([System.IO.Path]::Combine($(CoverletReport),'..'))` because the former would have crashed when `$(CoverletReport)` evaluates to empty, the latter just silently picked the wrong parent directory. But I did not use the former because of a [Rider bug][1]. 2. VS Test would not [create test results in non deterministic paths][2]. The `MoveCoverletReport` target would not have been needed at all. [1]: https://youtrack.jetbrains.com/issue/RIDER-92994 [2]: microsoft/vstest#2378
1 parent f48caec commit b85d9b7

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

tests/Serilog.Formatting.Log4Net.Tests.csproj

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,26 +44,26 @@
4444
</Target>
4545

4646
<!-- Because of https://github.com/microsoft/vstest/issues/2378 -->
47-
<Target Name="MoveCoverletReport" AfterTargets="GenerateHtmlCoverageReport">
47+
<Target Name="MoveCoverletReport" AfterTargets="GenerateHtmlCoverageReport" Condition="$(TargetFramework) != ''">
4848
<Move SourceFiles="@(CoverletReport)" DestinationFolder="$(CoverageReportDirectory)" />
4949
<PropertyGroup>
5050
<CoverletReport>@(CoverletReport)</CoverletReport>
5151
</PropertyGroup>
5252
<ItemGroup>
53-
<CoverletReportParentDirectory Include="$([System.IO.Path]::Combine($(CoverletReport),'..'))" />
53+
<CoverletReportParentDirectory Include="$([System.IO.Path]::Combine($(CoverletReport),'..'))" Condition="$(CoverletReport) != ''" />
5454
</ItemGroup>
5555
<RemoveDir Directories="@(CoverletReportParentDirectory)" />
5656
</Target>
5757

58-
<Target Name="DisplayCoverageSummary" AfterTargets="MoveCoverletReport">
58+
<Target Name="DisplayCoverageSummary" AfterTargets="MoveCoverletReport" Condition="$(TargetFramework) != ''">
5959
<PropertyGroup>
6060
<CatCommand Condition="!$([MSBuild]::IsOSPlatform('Windows'))">cat</CatCommand>
6161
<CatCommand Condition="$([MSBuild]::IsOSPlatform('Windows'))">type</CatCommand>
6262
</PropertyGroup>
6363
<Exec WorkingDirectory="$(CoverageReportDirectory)" Command="$(CatCommand) Summary.txt" />
6464
</Target>
6565

66-
<Target Name="OpenHtmlCoverageReport" AfterTargets="MoveCoverletReport" Condition="$(ContinuousIntegrationBuild) != 'true'">
66+
<Target Name="OpenHtmlCoverageReport" AfterTargets="MoveCoverletReport" Condition="$(TargetFramework) != '' AND $(ContinuousIntegrationBuild) != 'true'">
6767
<PropertyGroup>
6868
<OpenCommand Condition="$([MSBuild]::IsOSPlatform('Linux'))">xdg-open</OpenCommand>
6969
<OpenCommand Condition="$([MSBuild]::IsOSPlatform('OSX'))">open</OpenCommand>

0 commit comments

Comments
 (0)