Skip to content

Commit f33ee47

Browse files
test(ffe): use canonical FFE fixtures (#8616)
## Motivation Use the shared FFE fixture corpus so the .NET evaluator is checked against the same behavior as the other tracer implementations. This reduces fixture drift and gives us a repeatable way to expose and correct evaluator bugs when new canonical cases are added. The same fixture corpus is used by [Java](DataDog/dd-trace-java#11355), [libdatadog](DataDog/libdatadog#1979), [Go](DataDog/dd-trace-go#4753), [Python](DataDog/dd-trace-py#19390), [JavaScript](DataDog/dd-trace-js#8441), and [Ruby](DataDog/dd-trace-rb#5742). ## Changes - Replace the legacy copied fixtures with a generated, checked-in snapshot from `DataDog/ffe-system-test-data`. - Record the exact upstream commit in `SOURCE.md`. - Add a script that fetches, validates, and copies the canonical configuration and evaluation cases. - Add a weekly and manually dispatchable workflow that opens a signed draft dependency PR only when fixture contents have changed. - Parse flags independently so malformed flags do not reject valid neighbors. - Return `FLAG_NOT_FOUND` for missing flags and classify temporal, static, and split allocations. - Assert canonical values and reasons through the existing .NET unit-test suite. ## Fixture update flow When we add or change shared evaluator behavior, I imagine the flow working like this: 1. Add the new configuration and evaluation cases to [`DataDog/ffe-system-test-data`](https://github.com/DataDog/ffe-system-test-data) and review the expected behavior there. 2. The weekly updater, or a manually dispatched run for a specific ref, fetches the canonical repository and compares its fixture contents with the checked-in .NET snapshot. 3. If nothing changed, the workflow exits without opening or updating a PR. 4. If fixtures changed, the workflow copies them into this repository, records the source commit, and opens a signed draft PR with the normal dependency labels. 5. The .NET unit tests run against the updated cases. New tests may intentionally fail when they catch an evaluator bug or unsupported behavior. 6. Address those evaluator failures in the same dependency PR, keeping the fixture expectations unchanged unless the shared expectation itself is incorrect. 7. Merge the update once the .NET evaluator satisfies the new canonical cases. This keeps new behavior explicit and reviewable: fixture changes land in the canonical repository first, and each tracer then gets a visible compatibility PR rather than silently changing at build time. ## Decisions - `DataDog/ffe-system-test-data` remains the canonical source of shared evaluator behavior. - Keep the generated snapshot checked in so local and CI unit tests do not require network access or submodule initialization. - Use a scheduled dependency-update workflow instead of a git submodule. - Treat failures introduced by new canonical fixtures as useful regression signals and fix the evaluator as part of accepting the update. - Do not create a PR when the canonical fixture contents are unchanged, even if the upstream repository has unrelated commits.
1 parent b18aab8 commit f33ee47

69 files changed

Lines changed: 8242 additions & 6358 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/actions/create-signed-pull-request/action.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ inputs:
3030
reviewers:
3131
description: 'Comma-separated reviewers to request. GitHub usernames, not teams.'
3232
required: false
33+
draft:
34+
description: 'Whether to create the pull request as a draft.'
35+
required: false
36+
default: 'false'
3337

3438
outputs:
3539
pull-request-number:
@@ -55,4 +59,5 @@ runs:
5559
body: ${{ inputs.body }}
5660
labels: ${{ inputs.labels }}
5761
reviewers: ${{ inputs.reviewers }}
62+
draft: ${{ inputs.draft }}
5863
delete-branch: true
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
issuer: https://token.actions.githubusercontent.com
2+
3+
subject_pattern: repo:DataDog/dd-trace-dotnet:.*
4+
5+
claim_pattern:
6+
event_name: (schedule|workflow_dispatch)
7+
ref: refs/heads/master
8+
ref_protected: "true"
9+
job_workflow_ref: DataDog/dd-trace-dotnet/\.github/workflows/update_ffe_fixtures\.yml@refs/heads/master
10+
11+
permissions:
12+
contents: write
13+
pull_requests: write
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
name: Update FFE fixtures
2+
3+
on:
4+
schedule:
5+
- cron: "0 0 * * 0" # Every Sunday at midnight UTC
6+
workflow_dispatch:
7+
inputs:
8+
fixture_ref:
9+
description: Branch, tag, or commit from DataDog/ffe-system-test-data
10+
required: true
11+
default: main
12+
type: string
13+
14+
concurrency:
15+
group: update-ffe-fixtures
16+
cancel-in-progress: false
17+
18+
jobs:
19+
update_ffe_fixtures:
20+
runs-on: ubuntu-latest
21+
permissions:
22+
id-token: write
23+
24+
steps:
25+
- name: Get dd-octo-sts token
26+
uses: DataDog/dd-octo-sts-action@96a25462dbcb10ebf0bfd6e2ccc917d2ab235b9a # v1.0.4
27+
id: octo-sts
28+
with:
29+
scope: DataDog/dd-trace-dotnet
30+
policy: self.update_ffe_fixtures.create-pr
31+
32+
- name: Checkout
33+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
34+
with:
35+
persist-credentials: false
36+
37+
- name: Update fixture snapshot
38+
id: fixtures
39+
env:
40+
FFE_FIXTURE_REF: ${{ inputs.fixture_ref || 'main' }}
41+
run: ./tracer/build.sh UpdateFfeFixtures --FfeFixtureRef "$FFE_FIXTURE_REF"
42+
43+
- name: Create Pull Request
44+
if: steps.fixtures.outputs.changed == 'true'
45+
uses: ./.github/actions/create-signed-pull-request
46+
with:
47+
token: ${{ steps.octo-sts.outputs.token }}
48+
branch: bot/ffe-fixtures-update
49+
commit-message: "test(ffe): update canonical fixtures"
50+
base: master
51+
title: "test(ffe): update canonical fixtures"
52+
draft: true
53+
labels: "area:tests,feature_flags"
54+
body: |
55+
## Summary of changes
56+
57+
Updates the FFE tests to commit ${{ steps.fixtures.outputs.source_commit }}.
58+
59+
## Reason for change
60+
61+
Keep dd-trace-dotnet synchronized with the canonical FFE evaluator fixtures. New fixtures may intentionally make unit tests fail when they expose evaluator bugs that must be addressed before this update is merged.
62+
63+
Canonical source: https://github.com/DataDog/ffe-system-test-data
64+
Source commit: ${{ steps.fixtures.outputs.source_commit }}
65+
66+
## Implementation details
67+
68+
- Refresh the checked-in FFE fixture snapshot from the canonical repository.
69+
- Load ${{ steps.fixtures.outputs.fixture_count }} canonical JSON fixture cases.
70+
71+
## Test coverage
72+
73+
The existing FFE unit tests run against the refreshed canonical fixtures.
74+
75+
## Other details
76+
77+
Address evaluator failures in this update PR before merging it.
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.IO;
4+
using System.Linq;
5+
using System.Text.Json;
6+
using Nuke.Common;
7+
using Nuke.Common.IO;
8+
using Nuke.Common.Tooling;
9+
using static Nuke.Common.IO.FileSystemTasks;
10+
using Logger = Serilog.Log;
11+
12+
partial class Build
13+
{
14+
private static readonly HashSet<string> FfeFixtureCopyDisallowList = new(StringComparer.Ordinal)
15+
{
16+
".git",
17+
".github",
18+
".gitignore",
19+
"ci",
20+
"CONTRIBUTING.md",
21+
"LICENSE",
22+
"LICENSE-3rdparty.csv",
23+
"NOTICE",
24+
"README.md",
25+
"SOURCE.md",
26+
};
27+
28+
[Parameter("Branch, tag, or commit to copy from DataDog/ffe-system-test-data")]
29+
readonly string FfeFixtureRef = "main";
30+
31+
Target UpdateFfeFixtures => _ => _
32+
.Description("Updates the checked-in FFE fixtures from DataDog/ffe-system-test-data")
33+
.Executes(() =>
34+
{
35+
ValidateFixtureRef(FfeFixtureRef);
36+
37+
var destination = RootDirectory / "tracer" / "test" / "Datadog.Trace.Tests" / "FeatureFlags" / "ffe-system-test-data";
38+
var workingDirectory = TemporaryDirectory / $"ffe-system-test-data-{Guid.NewGuid():N}";
39+
var source = workingDirectory / "source";
40+
var snapshot = workingDirectory / "snapshot";
41+
var emptyGitConfig = workingDirectory / "empty-git-config";
42+
43+
try
44+
{
45+
EnsureExistingDirectory(source);
46+
EnsureExistingDirectory(snapshot);
47+
File.WriteAllText(emptyGitConfig, string.Empty);
48+
49+
var gitEnvironment = new Dictionary<string, string>
50+
{
51+
["GIT_CONFIG_NOSYSTEM"] = "1",
52+
["GIT_CONFIG_GLOBAL"] = emptyGitConfig,
53+
};
54+
55+
RunGit(source, "init --quiet", gitEnvironment);
56+
RunGit(source, "remote add origin https://github.com/DataDog/ffe-system-test-data.git", gitEnvironment);
57+
RunGit(source, $"fetch --quiet --depth 1 origin {FfeFixtureRef}", gitEnvironment);
58+
RunGit(source, "checkout --quiet --detach FETCH_HEAD", gitEnvironment);
59+
var sourceCommit = RunGit(source, "rev-parse HEAD", gitEnvironment);
60+
61+
CopyFixtureSnapshot(source, snapshot);
62+
var fixtureCount = ValidateFixtureSnapshot(snapshot);
63+
var changed = !HaveSameContents(snapshot, destination);
64+
65+
if (changed)
66+
{
67+
File.WriteAllText(
68+
snapshot / "SOURCE.md",
69+
$"""
70+
# FFE Fixture Snapshot
71+
72+
These files are copied from the canonical FFE fixture repository.
73+
74+
Canonical source: https://github.com/DataDog/ffe-system-test-data
75+
Source commit: {sourceCommit}
76+
77+
Do not edit these fixtures directly in dd-trace-dotnet. Add or update shared FFE behavior in ffe-system-test-data first, then refresh this snapshot.
78+
79+
The weekly update workflow runs `./tracer/build.sh UpdateFfeFixtures` and opens a draft test PR only when the allowed fixture contents change.
80+
""");
81+
82+
EnsureCleanDirectory(destination);
83+
CopyDirectory(snapshot, destination);
84+
}
85+
86+
Logger.Information("Checked FFE fixtures from DataDog/ffe-system-test-data@{SourceCommit}", sourceCommit);
87+
Logger.Information("Loaded {FixtureCount} JSON fixture cases", fixtureCount);
88+
Logger.Information("Fixture snapshot changed: {Changed}", changed);
89+
90+
var githubOutput = Environment.GetEnvironmentVariable("GITHUB_OUTPUT");
91+
if (!string.IsNullOrWhiteSpace(githubOutput))
92+
{
93+
File.AppendAllLines(
94+
githubOutput,
95+
new[]
96+
{
97+
$"source_commit={sourceCommit}",
98+
$"fixture_count={fixtureCount}",
99+
$"changed={changed.ToString().ToLowerInvariant()}",
100+
});
101+
}
102+
}
103+
finally
104+
{
105+
DeleteDirectory(workingDirectory);
106+
}
107+
});
108+
109+
private static void ValidateFixtureRef(string fixtureRef)
110+
{
111+
if (string.IsNullOrWhiteSpace(fixtureRef)
112+
|| fixtureRef.StartsWith("-", StringComparison.Ordinal)
113+
|| fixtureRef.Contains("..", StringComparison.Ordinal)
114+
|| fixtureRef.Any(character => !(char.IsLetterOrDigit(character) || character is '.' or '_' or '/' or '-')))
115+
{
116+
throw new ArgumentException($"Invalid FFE fixture ref: {fixtureRef}", nameof(fixtureRef));
117+
}
118+
}
119+
120+
private static string RunGit(AbsolutePath workingDirectory, string arguments, IReadOnlyDictionary<string, string> environment)
121+
{
122+
var process = ProcessTasks.StartProcess(
123+
"git",
124+
arguments,
125+
workingDirectory,
126+
environmentVariables: environment,
127+
logOutput: false);
128+
process.AssertZeroExitCode();
129+
return string.Join(Environment.NewLine, process.Output.Select(line => line.Text)).Trim();
130+
}
131+
132+
private static void CopyFixtureSnapshot(AbsolutePath source, AbsolutePath snapshot)
133+
{
134+
foreach (var entry in new DirectoryInfo(source).EnumerateFileSystemInfos())
135+
{
136+
if (FfeFixtureCopyDisallowList.Contains(entry.Name))
137+
{
138+
continue;
139+
}
140+
141+
CopyEntry(entry, Path.Combine(snapshot, entry.Name));
142+
}
143+
}
144+
145+
private static void CopyEntry(FileSystemInfo source, string destination)
146+
{
147+
if ((source.Attributes & FileAttributes.ReparsePoint) != 0)
148+
{
149+
throw new InvalidOperationException($"Refusing to copy symbolic link from FFE fixture repository: {source.FullName}");
150+
}
151+
152+
if (source is FileInfo file)
153+
{
154+
Directory.CreateDirectory(Path.GetDirectoryName(destination)!);
155+
file.CopyTo(destination, overwrite: true);
156+
return;
157+
}
158+
159+
Directory.CreateDirectory(destination);
160+
foreach (var child in ((DirectoryInfo)source).EnumerateFileSystemInfos())
161+
{
162+
CopyEntry(child, Path.Combine(destination, child.Name));
163+
}
164+
}
165+
166+
private static int ValidateFixtureSnapshot(AbsolutePath snapshot)
167+
{
168+
var configPath = snapshot / "ufc-config.json";
169+
var casesDirectory = snapshot / "evaluation-cases";
170+
if (!File.Exists(configPath) || !Directory.Exists(casesDirectory))
171+
{
172+
throw new InvalidOperationException("FFE fixture repository does not contain the expected fixture layout");
173+
}
174+
175+
using (var config = JsonDocument.Parse(File.ReadAllText(configPath)))
176+
{
177+
}
178+
179+
var caseFiles = Directory.GetFiles(casesDirectory, "*.json").OrderBy(path => path, StringComparer.Ordinal).ToArray();
180+
if (caseFiles.Length == 0)
181+
{
182+
throw new InvalidOperationException("No FFE JSON fixture files found");
183+
}
184+
185+
var fixtureCount = 0;
186+
foreach (var caseFile in caseFiles)
187+
{
188+
using var cases = JsonDocument.Parse(File.ReadAllText(caseFile));
189+
if (cases.RootElement.ValueKind != JsonValueKind.Array)
190+
{
191+
throw new InvalidOperationException($"{caseFile} must contain a JSON array of test cases");
192+
}
193+
194+
fixtureCount += cases.RootElement.GetArrayLength();
195+
}
196+
197+
if (fixtureCount == 0)
198+
{
199+
throw new InvalidOperationException("No FFE fixture test cases found");
200+
}
201+
202+
return fixtureCount;
203+
}
204+
205+
private static bool HaveSameContents(AbsolutePath snapshot, AbsolutePath destination)
206+
{
207+
if (!Directory.Exists(destination))
208+
{
209+
return false;
210+
}
211+
212+
var snapshotFiles = GetRelativeFiles(snapshot);
213+
var destinationFiles = GetRelativeFiles(destination, "SOURCE.md");
214+
if (!snapshotFiles.SequenceEqual(destinationFiles, StringComparer.Ordinal))
215+
{
216+
return false;
217+
}
218+
219+
return snapshotFiles.All(relativePath =>
220+
File.ReadAllBytes(snapshot / relativePath).AsSpan().SequenceEqual(File.ReadAllBytes(destination / relativePath)));
221+
}
222+
223+
private static string[] GetRelativeFiles(AbsolutePath directory, params string[] excludedFiles)
224+
{
225+
return Directory.GetFiles(directory, "*", SearchOption.AllDirectories)
226+
.Select(path => Path.GetRelativePath(directory, path))
227+
.Where(path => !excludedFiles.Contains(path, StringComparer.Ordinal))
228+
.OrderBy(path => path, StringComparer.Ordinal)
229+
.ToArray();
230+
}
231+
232+
private static void CopyDirectory(AbsolutePath source, AbsolutePath destination)
233+
{
234+
foreach (var entry in new DirectoryInfo(source).EnumerateFileSystemInfos())
235+
{
236+
CopyEntry(entry, Path.Combine(destination, entry.Name));
237+
}
238+
}
239+
}

0 commit comments

Comments
 (0)