test(csharp): add E2E tests for FeatureFlagCache (Phase 7 E2E GATE)

[jadewang-db]

🥞 Stacked PR

Use this link to review incremental changes.

• stack/wi-3.5-metrics-aggregator [Files changed]
  • stack/wi-3.5-circuit-breaker-manager [Files changed]
    • stack/e2e-feature-flag-cache-tests [Files changed]
      • stack/databricks-activity-listener [Files changed]
        • stack/circuit-breaker-telemetry-exporter [Files changed]
          • stack/telemetry-client-manager-wi-3.2 [Files changed]
            • stack/telemetry-client-wi-5.5 [Files changed]
              • stack/telemetry-client-manager-e2e-wi-7 [Files changed]
                • stack/telemetry-client-e2e-tests-wi-7-standalone [Files changed]
                  • stack/wi-6.1-databricks-connection-telemetry-integration [Files changed]
                  • stack/wi-6.2-telemetry-tags-driver-activities [Files changed]
                  • stack/wi-9-full-integration-e2e-tests [Files changed]

---

[jadewang-db]

Code Review: FeatureFlagCacheE2ETests.cs

Executive Summary

Critical Issue Found: The E2E tests are testing methods that do not exist in the production FeatureFlagCache class. This code will not compile.

---

Critical Issues (Must Fix)

1. API Mismatch - Tests reference non-existent methods (Severity: Critical)

The test file imports AdbcDrivers.Databricks.Telemetry (line 21) but the production FeatureFlagCache is in AdbcDrivers.Databricks namespace.

More critically, the tests call methods that do not exist in the production code:

Test Method Used	Production Code	Status
cache.GetOrCreateContext(host)	GetOrCreateContextAsync(host, properties, driverVersion, ...) - private	❌ Different signature, private method
cache.IsTelemetryEnabledAsync()	N/A	❌ Does not exist
cache.ReleaseContext(host)	RemoveContext(host)	❌ Wrong method name
context.RefCount	N/A	❌ Does not exist
context.TelemetryEnabled	N/A	❌ Does not exist
context.LastFetched	N/A	❌ Does not exist
context.IsExpired	N/A	❌ Does not exist

The production FeatureFlagCache uses:

• IMemoryCache for TTL-based eviction (not reference counting)
• MergePropertiesWithFeatureFlagsAsync() as the main API
• FeatureFlagContext with GetFlagValue() and GetAllFlags()

2. Constructor signature mismatch (Severity: Critical)

Test creates cache with TTL: new FeatureFlagCache(TimeSpan.FromMinutes(15)) (line 197)

But production constructors are:

• FeatureFlagCache() - default
• FeatureFlagCache(IMemoryCache cache) - for testing with custom cache

---

Recommendations

1. Option A: Update the FeatureFlagCache implementation to support the test-facing API (reference counting, IsTelemetryEnabledAsync, etc.)

2. Option B: Rewrite the tests to match the actual production API:

   • Test MergePropertiesWithFeatureFlagsAsync() instead of IsTelemetryEnabledAsync()
   • Use IMemoryCache mock for cache testing
   • Remove reference counting tests (not part of current design)

3. Option C: If this is a planned future API, document it and implement the missing methods first

---

Questions for Author

1. Was this test written against a planned API that hasn't been implemented yet?
2. Should the FeatureFlagCache be refactored to use reference counting instead of IMemoryCache-based TTL eviction?

---

Overall Assessment

⛔ Cannot approve - The tests will fail to compile. The entire test file needs to be rewritten to match the actual production API, or the production code needs to be updated to support this API design.