Skip to content

Commit 3b8c367

Browse files
tonyredondotonyredondo
committed
Harden coverage backfill publish gating
1 parent 1497c38 commit 3b8c367

11 files changed

Lines changed: 304 additions & 194 deletions

File tree

‎tracer/src/Datadog.Trace.Tools.Runner/ConfigureCiCommand.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ private static bool TryExtractCiName(string name, out CIName? ciName)
6262
return false;
6363
}
6464

65-
private static void RemoveRunScopedBackfillEnvironmentVariables(Dictionary<string, string> environmentVariables)
65+
private static void RemoveRunScopedEnvironmentVariables(Dictionary<string, string> environmentVariables)
6666
{
6767
environmentVariables.Remove(ConfigurationKeys.CIVisibility.TestOptimizationRunId);
68+
environmentVariables.Remove(ConfigurationKeys.CIVisibility.TestSessionCommand);
69+
environmentVariables.Remove(ConfigurationKeys.CIVisibility.TestSessionWorkingDirectory);
6870
environmentVariables.Remove(ConfigurationKeys.CIVisibilityItrCoverageBackfillActualSkip);
6971
environmentVariables.Remove(ConfigurationKeys.CIVisibilityItrCoverageBackfillCommand);
7072
environmentVariables.Remove(ConfigurationKeys.CIVisibilityItrCoverageBackfillPath);
@@ -91,7 +93,7 @@ private async Task ExecuteAsync(InvocationContext context)
9193
}
9294

9395
AnsiConsole.WriteLine("Setting up the environment variables.");
94-
RemoveRunScopedBackfillEnvironmentVariables(initResults.ProfilerEnvironmentVariables);
96+
RemoveRunScopedEnvironmentVariables(initResults.ProfilerEnvironmentVariables);
9597

9698
if (!CIConfiguration.SetupCIEnvironmentVariables(initResults.ProfilerEnvironmentVariables, ciName))
9799
{

‎tracer/src/Datadog.Trace/Ci/Coverage/Backfill/CoverageBackfillApplicator.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,13 @@ public static CoverageBackfillResult ApplyToGlobalCoverage(GlobalCoverageInfo? g
6868

6969
var backendFileCount = CountBackendFilesWithCoverage(backfillData);
7070
var matchedFiles = 0;
71+
var publishableBackendKeys = new HashSet<string>(StringComparer.Ordinal);
7172
foreach (var backendKey in matchedBackendKeys)
7273
{
7374
if (!unsafeBackendKeys.Contains(backendKey))
7475
{
7576
matchedFiles++;
77+
publishableBackendKeys.Add(backendKey);
7678
}
7779
}
7880

@@ -98,7 +100,21 @@ public static CoverageBackfillResult ApplyToGlobalCoverage(GlobalCoverageInfo? g
98100
matchedFiles,
99101
updatedFiles,
100102
hasBackendCoverage: backendFileCount > 0,
101-
canPublishCoverage: true);
103+
canPublishCoverage: CanPublishBackfilledCoverage(backfillData, publishableBackendKeys));
104+
}
105+
106+
private static bool CanPublishBackfilledCoverage(CoverageBackfillData backfillData, HashSet<string> publishableBackendKeys)
107+
{
108+
foreach (var item in backfillData.ExecutedLinesByRelativePath)
109+
{
110+
if (HasActiveBits(item.Value) &&
111+
!publishableBackendKeys.Contains(item.Key))
112+
{
113+
return false;
114+
}
115+
}
116+
117+
return true;
102118
}
103119

104120
private static bool TryGetMergedExecutedBitmap(FileCoverageInfo file, byte[] backendExecutedBitmap, out byte[] mergedBitmap)

‎tracer/src/Datadog.Trace/Ci/Coverage/Backfill/ExternalCoverageXmlBackfill.cs‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,14 +2250,51 @@ private static bool CanPublishBackfilledCoverage(CoverageBackfillData? backfillD
22502250
return matchedBackendPaths.Count == 0;
22512251
}
22522252

2253-
foreach (var backendPath in matchedBackendPaths)
2253+
var backendFileCount = 0;
2254+
foreach (var item in backfillData.ExecutedLinesByRelativePath)
22542255
{
2255-
if (!backfillData.ExecutedLinesByRelativePath.ContainsKey(backendPath))
2256+
if (!HasActiveBits(item.Value))
2257+
{
2258+
continue;
2259+
}
2260+
2261+
backendFileCount++;
2262+
if (!matchedBackendPaths.Contains(item.Key) ||
2263+
!representedBackendLines.TryGetValue(item.Key, out var representedLines) ||
2264+
!RepresentsAllActiveBackendLines(item.Value, representedLines))
22562265
{
22572266
return false;
22582267
}
22592268
}
22602269

2270+
return backendFileCount == matchedBackendPaths.Count;
2271+
}
2272+
2273+
private static bool RepresentsAllActiveBackendLines(byte[] bitmap, HashSet<int> representedLines)
2274+
{
2275+
for (var byteIndex = 0; byteIndex < bitmap.Length; byteIndex++)
2276+
{
2277+
var value = bitmap[byteIndex];
2278+
if (value == 0)
2279+
{
2280+
continue;
2281+
}
2282+
2283+
for (var bitIndex = 0; bitIndex < 8; bitIndex++)
2284+
{
2285+
if ((value & (128 >> bitIndex)) == 0)
2286+
{
2287+
continue;
2288+
}
2289+
2290+
var line = (byteIndex * 8) + bitIndex + 1;
2291+
if (!representedLines.Contains(line))
2292+
{
2293+
return false;
2294+
}
2295+
}
2296+
}
2297+
22612298
return true;
22622299
}
22632300

@@ -3407,6 +3444,17 @@ internal bool CanPublish()
34073444
!_unsupportedBackfill &&
34083445
CanPublishBackfilledCoverage(_backfillData, _matchedBackendPaths, _representedBackendLines));
34093446

3447+
/// <summary>
3448+
/// Gets whether this report set can be kept after local XML backfill when it will not be used for coverage publication.
3449+
/// </summary>
3450+
/// <returns>True when backfill either was not attempted or represented at least one backend line without unsafe matches.</returns>
3451+
internal bool CanKeepUnpublishedBackfill()
3452+
=> !_backfillAttempted ||
3453+
(!_unsafePathMatch &&
3454+
!_duplicateRepresentedBackendLine &&
3455+
!_unsupportedBackfill &&
3456+
RepresentedBackendLineCount > 0);
3457+
34103458
internal void MarkBackfillAttempted(CoverageBackfillData backfillData)
34113459
{
34123460
_backfillAttempted = true;

‎tracer/src/Datadog.Trace/Ci/TestOptimizationSkippableFeature.cs‎

Lines changed: 11 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ internal sealed class TestOptimizationSkippableFeature : ITestOptimizationSkippa
3434
private readonly Dictionary<string, ActualSkippedDictionaryState> _actualSkippedDictionaries = new(StringComparer.Ordinal);
3535
private readonly ConcurrentDictionary<ulong, byte> _itrSkippedSessions = new();
3636
private readonly bool _coverageBackfillRequiresScopedRequests;
37+
private readonly string _coverageBackfillUnsupportedReason;
3738
// Keeps scoped tasks in insertion order so shutdown can wait for them without allocating array snapshots.
3839
private List<Task<SkippableTestsDictionary>>? _scopedSkippableTestsTaskList;
3940

@@ -48,6 +49,10 @@ private TestOptimizationSkippableFeature(TestOptimizationSettings settings, Test
4849
}
4950

5051
_coverageBackfillRequiresScopedRequests = CoverageBackfillCapability.IsCoverageBackfillRequired(settings);
52+
_coverageBackfillUnsupportedReason = _coverageBackfillRequiresScopedRequests &&
53+
!CoverageBackfillCapability.IsActiveCoverageModeBackfillable(settings, out var unsupportedReason) ?
54+
unsupportedReason :
55+
string.Empty;
5156
if (settings.TestsSkippingEnabled == true)
5257
{
5358
Log.Information("TestOptimizationSkippableFeature: Test skipping is enabled.");
@@ -226,8 +231,7 @@ public CoverageBackfillData GetCoverageBackfillData()
226231
{
227232
if (_coverageBackfillRequiresScopedRequests)
228233
{
229-
var completedScopedBackfillData = GetCompletedScopedCoverageBackfillData();
230-
return completedScopedBackfillData.IsPresent ? completedScopedBackfillData : GetActualSkippedCoverageBackfillData();
234+
return GetActualSkippedCoverageBackfillData();
231235
}
232236

233237
return TryGetUnscopedSkippableTests(out var skippableTestsBySuiteAndName) ? skippableTestsBySuiteAndName.BackfillData : CoverageBackfillData.Missing;
@@ -244,7 +248,7 @@ public bool IsCoverageBackfillSafe()
244248
{
245249
if (_coverageBackfillRequiresScopedRequests)
246250
{
247-
return AreCompletedScopedDictionariesCoverageBackfillSafe() || AreActualSkippedDictionariesCoverageBackfillSafe();
251+
return AreActualSkippedDictionariesCoverageBackfillSafe();
248252
}
249253

250254
return TryGetUnscopedSkippableTests(out var skippableTestsBySuiteAndName) && IsDictionaryCoverageBackfillSafe(skippableTestsBySuiteAndName);
@@ -259,16 +263,15 @@ public bool CanSkipWithCoverageBackfill(SkippableTest skippableTest, string? mod
259263
return true;
260264
}
261265

262-
if (skippableTest.MissingLineCodeCoverage == true)
266+
if (!StringUtil.IsNullOrEmpty(_coverageBackfillUnsupportedReason))
263267
{
264-
reason = "backend marked the test as missing line coverage";
268+
reason = _coverageBackfillUnsupportedReason;
265269
return false;
266270
}
267271

268-
if (TryGetCoverageBackfillDictionary(moduleName, out var skippableTestsBySuiteAndName) &&
269-
!IsDictionaryCoverageBackfillSafe(skippableTestsBySuiteAndName))
272+
if (skippableTest.MissingLineCodeCoverage == true)
270273
{
271-
reason = "backend coverage data is unavailable or unsafe";
274+
reason = "backend marked the test as missing line coverage";
272275
return false;
273276
}
274277

@@ -497,57 +500,6 @@ private bool HasCompletedScopedSkippableTests()
497500
return false;
498501
}
499502

500-
/// <summary>
501-
/// Gets merged backend coverage from completed scoped skippable responses without requiring actual skip state.
502-
/// </summary>
503-
/// <returns>The merged coverage backfill data, or missing coverage data when no completed scoped response has usable coverage.</returns>
504-
private CoverageBackfillData GetCompletedScopedCoverageBackfillData()
505-
{
506-
lock (_scopedSkippableTestsTasks)
507-
{
508-
if (_scopedSkippableTestsTaskList is not { Count: > 0 } scopedTasks)
509-
{
510-
return CoverageBackfillData.Missing;
511-
}
512-
513-
var coverageMaps = new List<CoverageBackfillData>(scopedTasks.Count);
514-
foreach (var task in scopedTasks)
515-
{
516-
if (task.Status == TaskStatus.RanToCompletion && IsDictionaryCoverageBackfillSafe(task.Result))
517-
{
518-
coverageMaps.Add(task.Result.BackfillData);
519-
}
520-
}
521-
522-
return coverageMaps.Count == 0 ? CoverageBackfillData.Missing : CoverageBackfillData.Merge(coverageMaps);
523-
}
524-
}
525-
526-
/// <summary>
527-
/// Checks completed scoped skippable responses for usable backend coverage without requiring actual skip state.
528-
/// </summary>
529-
/// <returns><c>true</c> when any completed scoped response has usable backend coverage; otherwise, <c>false</c>.</returns>
530-
private bool AreCompletedScopedDictionariesCoverageBackfillSafe()
531-
{
532-
lock (_scopedSkippableTestsTasks)
533-
{
534-
if (_scopedSkippableTestsTaskList is not { Count: > 0 } scopedTasks)
535-
{
536-
return false;
537-
}
538-
539-
foreach (var task in scopedTasks)
540-
{
541-
if (task.Status == TaskStatus.RanToCompletion && IsDictionaryCoverageBackfillSafe(task.Result))
542-
{
543-
return true;
544-
}
545-
}
546-
}
547-
548-
return false;
549-
}
550-
551503
/// <summary>
552504
/// Gets the completed scoped correlation id for a local module without creating a late backend request.
553505
/// </summary>

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/DotnetTest/DotnetCommon.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ private static void TryProcessUnselectedCoverletCollectorXmlReportGroup(
827827
validationState.Merge(reportValidationState);
828828
}
829829

830-
if (!validationState.CanPublish())
830+
if (!validationState.CanKeepUnpublishedBackfill())
831831
{
832832
TryRestoreCoverageXmlReports(backups);
833833
Log.Debug("RunCiCommand: Unselected Coverlet collector XML report set could not be safely backfilled, so it was restored.");

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/Testing/DotnetTest/ManagedVanguardStopIntegration.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,14 @@ internal static CallTargetReturn OnMethodEnd<TTarget>(TTarget instance, Exceptio
7171
}
7272

7373
var backfillData = CoverageBackfillData.Missing;
74-
var shouldBackfill = DotnetCommon.TryGetCoverageBackfillDataForCurrentProcess(parentSessionSpanId, out backfillData);
74+
var shouldBackfill = DotnetCommon.TryGetCoverageBackfillDataForCurrentProcess(parentSessionSpanId, out backfillData, out var unavailableAfterActualItrSkip);
75+
if (!shouldBackfill && unavailableAfterActualItrSkip)
76+
{
77+
DotnetCommon.Log.Warning("MicrosoftCodeCoverage: ITR skipped tests but backend coverage backfill data is unavailable, so no stale coverage percentage will be sent.");
78+
RecordMicrosoftCoverageIpcFailure(parentSessionSpanId);
79+
return CallTargetReturn.GetDefault();
80+
}
81+
7582
var coverageResults = new List<ExternalCoverageXmlResult>(coverageFiles.Count);
7683
var processedCoverageFiles = new List<string>(coverageFiles.Count);
7784
var coverageReportBackups = shouldBackfill ? new List<CoverageXmlReportBackup>(coverageFiles.Count) : null;

‎tracer/test/Datadog.Trace.Tests/Ci/CoverageBackfillDataTests.cs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ public void GlobalCoverageBackfillMatchesUniqueCaseInsensitiveSuffixOnlyOnWindow
447447
else
448448
{
449449
result.MatchedFiles.Should().Be(0);
450-
result.CanPublishCoverage.Should().BeTrue();
450+
result.CanPublishCoverage.Should().BeFalse();
451451
result.UpdatedFiles.Should().Be(0);
452452
globalCoverage.Components[0].Files[0].ExecutedBitmap.Should().Equal([0b_0000_0000]);
453453
}
@@ -487,7 +487,7 @@ public void GlobalCoverageBackfillDoesNotSuffixMatchAbsolutePathOutsideSourceRoo
487487

488488
result.Applied.Should().BeTrue();
489489
result.HasBackendCoverage.Should().BeTrue();
490-
result.CanPublishCoverage.Should().BeTrue();
490+
result.CanPublishCoverage.Should().BeFalse();
491491
result.Backfilled.Should().BeFalse();
492492
result.MatchedFiles.Should().Be(0);
493493
result.UpdatedFiles.Should().Be(0);
@@ -526,7 +526,7 @@ public void GlobalCoverageBackfillDoesNotSuffixMatchAbsolutePathWhenSourceRootIs
526526

527527
result.Applied.Should().BeTrue();
528528
result.HasBackendCoverage.Should().BeTrue();
529-
result.CanPublishCoverage.Should().BeTrue();
529+
result.CanPublishCoverage.Should().BeFalse();
530530
result.Backfilled.Should().BeFalse();
531531
result.MatchedFiles.Should().Be(0);
532532
result.UpdatedFiles.Should().Be(0);
@@ -567,7 +567,7 @@ public void GlobalCoverageBackfillDoesNotSuffixMatchRelativePathWithParentDirect
567567

568568
result.Applied.Should().BeTrue();
569569
result.HasBackendCoverage.Should().BeTrue();
570-
result.CanPublishCoverage.Should().BeTrue();
570+
result.CanPublishCoverage.Should().BeFalse();
571571
result.Backfilled.Should().BeFalse();
572572
result.MatchedFiles.Should().Be(0);
573573
result.UpdatedFiles.Should().Be(0);
@@ -609,7 +609,7 @@ public void GlobalCoverageBackfillIgnoresAmbiguousNonExactBackendKeyMatches()
609609

610610
result.Applied.Should().BeTrue();
611611
result.HasBackendCoverage.Should().BeTrue();
612-
result.CanPublishCoverage.Should().BeTrue();
612+
result.CanPublishCoverage.Should().BeFalse();
613613
result.Backfilled.Should().BeFalse();
614614
result.MatchedFiles.Should().Be(0);
615615
result.UpdatedFiles.Should().Be(0);
@@ -652,7 +652,7 @@ public void GlobalCoverageBackfillIgnoresBackendKeyWhenExactAndDistinctSuffixLoc
652652

653653
result.Applied.Should().BeTrue();
654654
result.HasBackendCoverage.Should().BeTrue();
655-
result.CanPublishCoverage.Should().BeTrue();
655+
result.CanPublishCoverage.Should().BeFalse();
656656
result.Backfilled.Should().BeFalse();
657657
result.MatchedFiles.Should().Be(0);
658658
result.UpdatedFiles.Should().Be(0);
@@ -817,7 +817,7 @@ public void GlobalCoverageBackfillRejectsAbsoluteUriPathBeforeSuffixMatch()
817817

818818
result.Applied.Should().BeTrue();
819819
result.HasBackendCoverage.Should().BeTrue();
820-
result.CanPublishCoverage.Should().BeTrue();
820+
result.CanPublishCoverage.Should().BeFalse();
821821
result.Backfilled.Should().BeFalse();
822822
result.MatchedFiles.Should().Be(0);
823823
result.UpdatedFiles.Should().Be(0);
@@ -907,7 +907,7 @@ public void PathMatcherRejectsAmbiguousCaseInsensitiveExactPaths()
907907
}
908908

909909
[Fact]
910-
public void GlobalCoverageBackfillIgnoresBackendOnlyFiles()
910+
public void GlobalCoverageBackfillBlocksPublishWhenBackendOnlyFilesAreActive()
911911
{
912912
var globalCoverage = new GlobalCoverageInfo
913913
{
@@ -936,14 +936,15 @@ public void GlobalCoverageBackfillIgnoresBackendOnlyFiles()
936936

937937
result.Applied.Should().BeTrue();
938938
result.HasBackendCoverage.Should().BeTrue();
939-
result.CanPublishCoverage.Should().BeTrue();
939+
result.CanPublishCoverage.Should().BeFalse();
940+
result.Backfilled.Should().BeFalse();
940941
result.MatchedFiles.Should().Be(0);
941942
result.UpdatedFiles.Should().Be(0);
942943
globalCoverage.GetTotalPercentage().Should().Be(100);
943944
}
944945

945946
[Fact]
946-
public void GlobalCoverageBackfillAppliesMatchedBackendCoverageAndIgnoresBackendOnlyFiles()
947+
public void GlobalCoverageBackfillBlocksPublishWhenAnyBackendActiveFileIsMissing()
947948
{
948949
var globalCoverage = new GlobalCoverageInfo
949950
{
@@ -973,15 +974,15 @@ public void GlobalCoverageBackfillAppliesMatchedBackendCoverageAndIgnoresBackend
973974

974975
result.Applied.Should().BeTrue();
975976
result.HasBackendCoverage.Should().BeTrue();
976-
result.CanPublishCoverage.Should().BeTrue();
977-
result.Backfilled.Should().BeTrue();
977+
result.CanPublishCoverage.Should().BeFalse();
978+
result.Backfilled.Should().BeFalse();
978979
result.MatchedFiles.Should().Be(1);
979980
result.UpdatedFiles.Should().Be(1);
980981
globalCoverage.Components[0].Files[0].ExecutedBitmap.Should().Equal([0b_1000_0000]);
981982
}
982983

983984
[Fact]
984-
public void GlobalCoverageBackfillCreatesExecutedBitmapForMatchedCoverageAndIgnoresBackendOnlyFiles()
985+
public void GlobalCoverageBackfillCreatesExecutedBitmapForMatchedCoverage()
985986
{
986987
var globalCoverage = new GlobalCoverageInfo
987988
{
@@ -1003,8 +1004,7 @@ public void GlobalCoverageBackfillCreatesExecutedBitmapForMatchedCoverageAndIgno
10031004
var backfill = CoverageBackfillData.FromBackendCoverage(
10041005
new Dictionary<string, string>
10051006
{
1006-
["src/Calculator.cs"] = Convert.ToBase64String([0b_1000_0000]),
1007-
["src/Other.cs"] = Convert.ToBase64String([0b_1000_0000])
1007+
["src/Calculator.cs"] = Convert.ToBase64String([0b_1000_0000])
10081008
});
10091009

10101010
var result = CoverageBackfillApplicator.ApplyToGlobalCoverage(globalCoverage, backfill, CIEnvironmentValues.Instance);

0 commit comments

Comments
 (0)