test(metrics): eliminate coverage theater with exact value assertions (#147)

* test(metrics): eliminate coverage theater with exact value assertions

- Extract MetricCollectionHarness to shared file (remove 2 duplicate copies)
- Create OTelTestHelper for InMemoryExporter-based unit tests
- Strengthen MultipleNamedCaches: assert exact per-cache hit/miss counts
- Strengthen WithCacheName: assert ALL measurements have correct tag
- Strengthen EstimatedSize: assert exact value (42) not just non-empty
- Strengthen EvictionAfterDisposal: assert counter NOT incremented post-dispose
- Strengthen MeterFactory: verify metrics emitted through factory meter
- Strengthen AdditionalTags: assert tag contents (whitespace excluded, duplicates excluded)
- Add OpenTelemetry.Exporter.InMemory to Unit.csproj
- Net -417 lines (deduplication + stronger assertions)

* fix(test): address PR #147 review findings

- Delete dead OTelTestHelper.cs (zero callers, YAGNI)
- Remove 3 unused OTel packages from Unit.csproj
- Fix double-lookup in MetricCollectionHarness (TryGetValue)
- Fix unnecessary allocation in GetMeasurements (Array.Empty)
- Make Collect() atomic (lock spans clear+record)
- Replace Task.Yield() with Task.Delay(10, CancellationToken.None)
- Remove duplicate assertions in 3 test methods
- Add using to all MeteredMemoryCache instances (12 total)
- Fix typo in XML doc comment

All 226 tests pass (208 unit + 18 integration).

* fix: remove unused OTelTestHelper and fix vacuously true eviction test

- Delete unused OTelTestHelper.cs class (dead code with no callers)
- Remove unnecessary OpenTelemetry NuGet packages that supported it
- Fix MeteredMemoryCache_EvictionCallback_AfterDisposal_NoMetricUpdate test:
  - Use CancellationChangeToken to trigger TokenExpired eviction
  - TokenExpired IS counted by _evictionCount, unlike Remove
  - This properly tests the disposal guard (the original test using
    inner.Remove triggered EvictionReason.Removed which is excluded
    from the counter regardless of disposal guard)

* fix(tests): Add Assert.NotEmpty guard in AssertAllMeasurementsHaveTags

Fix vacuous assertion that would pass when no measurements exist. The
Assert.All call passes trivially on an empty collection, allowing a
broken implementation emitting zero 'cache.requests' measurements to
pass the test.

Added Assert.NotEmpty(measurements) guard before Assert.All to ensure
the test fails if no measurements are emitted, consistent with the
pattern used in CoverageGapTests.cs for AdditionalTags tests.

* fix(test): address PR #147 review findings (round 2)

- Eliminate coverage theater: replace Assert.Contains(true,...) with AssertAllMeasurementsHaveTags in 3 shared test methods
- Fix double-lookup in GetMeasurementsWithTags (TryGetValue pattern)
- Remove DRY violation: AssertMeasurementCount delegates to AssertAggregatedCount
- Add AggregateException catch in Collect() for post-disposal safety
- Improve comment accuracy for type-mismatch and eviction scenarios

226 tests pass (208 unit + 18 integration), 0 warnings.

* fix(tests): replace remaining Assert.Contains(true,...) theater in MeteredMemoryCacheTests

Boy Scout Rule: 6 instances of Assert.Contains(true, ...Select(m => m.Tags.Any(...)))
verified 'any measurement has tag' instead of 'ALL measurements have tag'.

Added AssertAllMeasurementsHaveTag to TestListener with NotEmpty guard.
Replaced all 6 instances across 4 tests:
- WithCacheName_EmitsCacheNameTag
- OptionsConstructor_WithAdditionalTags_EmitsAllTagsOnMetrics
- TryGetValue_Generic_WithNamedCache_RecordsMetricsWithCacheName
- GetOrCreate_WithNamedCache_RecordsMetricsWithCacheName

Zero Assert.Contains(true,...) patterns remain in the test suite.

---------

Co-authored-by: Richard Murillo <rjmurillo@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>

Author: rjmurillo

tests/Unit/CoverageGapTests.cs

@@ -132,7 +132,7 @@ public void MeteredMemoryCache_WithTrackStatistics_ReportsEstimatedSize()
 
         listener.RecordObservableInstruments();
 
-        Assert.NotEmpty(measurements);
+        Assert.Contains(42L, measurements);
     }
 
     #endregion
@@ -219,9 +219,16 @@ public void MeteredMemoryCache_NonNullMeterFactory_UsesFactoryMeter()
         using var cache = new MeteredMemoryCache(inner, meterFactory, cacheName);
 
         Assert.Equal(cacheName, cache.Name);
-        cache.Set("key", "value");
-        Assert.True(cache.TryGetValue("key", out var val));
-        Assert.Equal("value", val);
+
+        // Perform operations that should emit metrics
+        cache.TryGetValue("miss-key", out _); // miss
+        cache.Set("hit-key", "value");
+        cache.TryGetValue("hit-key", out _); // hit
+
+        // Verify metrics were emitted through the factory-created meter
+        var stats = cache.GetCurrentStatistics();
+        Assert.Equal(1, stats.TotalHits);
+        Assert.Equal(1, stats.TotalMisses);
     }
 
     [Fact]
@@ -250,21 +257,52 @@ public void MeteredMemoryCache_EvictionCallback_AfterDisposal_NoMetricUpdate()
         using var meter = new Meter(SharedUtilities.GetUniqueMeterName("evict-after-dispose"));
         var cacheName = SharedUtilities.GetUniqueCacheName("evict-disposed");
 
+        long evictionCount = 0;
+        using var listener = new MeterListener();
+        listener.InstrumentPublished = (inst, ml) =>
+        {
+            if (inst.Meter.Name == meter.Name && inst.Name == "cache.evictions")
+            {
+                ml.EnableMeasurementEvents(inst);
+            }
+        };
+        listener.SetMeasurementEventCallback<long>((_, value, _, _) =>
+        {
+            evictionCount += value;
+        });
+        listener.Start();
+
         var cache = new MeteredMemoryCache(inner, meter, cacheName);
 
-        // Add entry then dispose cache before eviction fires
+        // Use CancellationChangeToken to trigger TokenExpired eviction (which IS counted)
+        // Unlike Remove/Replaced, TokenExpired would increment _evictionCount without the disposal guard
+        var cts = new CancellationTokenSource();
+
+        // Add entry with expiration token, then dispose cache before eviction fires
         using (var entry = cache.CreateEntry("key"))
         {
             entry.Value = "value";
+            entry.AddExpirationToken(new Microsoft.Extensions.Primitives.CancellationChangeToken(cts.Token));
         }
 
         cache.Dispose();
 
-        // Force eviction by removing from inner cache — triggers the post-eviction callback
-        // with the disposal guard active
-        inner.Remove("key");
+        // Capture eviction count before forcing eviction
+        listener.RecordObservableInstruments();
+        var countBeforeEviction = evictionCount;
+
+        // Force eviction via token cancellation (triggers TokenExpired, which IS counted)
+        // The disposal guard should prevent the counter increment
+        cts.Cancel();
 
-        // The eviction callback disposal guard should have returned early — no throw
+        // Compact to process eviction callbacks
+        inner.Compact(1.0);
+
+        // Re-read Observable instruments
+        listener.RecordObservableInstruments();
+
+        // Eviction counter should NOT have increased after disposal due to disposal guard
+        Assert.Equal(countBeforeEviction, evictionCount);
     }
 
     #endregion
@@ -288,9 +326,38 @@ public void MeteredMemoryCache_AdditionalTags_EmptyKeyAfterTrim_Excluded()
             },
         };
 
-        // Should not throw - empty keys after trim are silently excluded
+        var tagSets = new List<KeyValuePair<string, object?>[]>();
+        using var listener = new MeterListener();
+        listener.InstrumentPublished = (inst, ml) =>
+        {
+            if (inst.Meter.Name == meter.Name)
+            {
+                ml.EnableMeasurementEvents(inst);
+            }
+        };
+        listener.SetMeasurementEventCallback<long>((inst, value, tags, _) =>
+        {
+            tagSets.Add(tags.ToArray());
+        });
+        listener.Start();
+
         using var cache = new MeteredMemoryCache(inner, meter, options);
-        Assert.Equal(cacheName, cache.Name);
+
+        // Perform operation to emit metrics
+        cache.Set("key", "val");
+        cache.TryGetValue("key", out _);
+
+        // Record Observable instruments
+        listener.RecordObservableInstruments();
+
+        // Verify whitespace key is NOT in any tag set
+        Assert.NotEmpty(tagSets);
+        Assert.All(tagSets, tags =>
+            Assert.DoesNotContain(tags, t => t.Key.Trim() == ""));
+
+        // Verify valid-tag IS present
+        Assert.Contains(tagSets, tags =>
+            tags.Any(t => t.Key == "valid-tag" && (string?)t.Value == "value"));
     }
 
     [Fact]
@@ -310,8 +377,43 @@ public void MeteredMemoryCache_AdditionalTags_CacheNameDuplicate_Excluded()
             },
         };
 
+        var tagSets = new List<KeyValuePair<string, object?>[]>();
+        using var listener = new MeterListener();
+        listener.InstrumentPublished = (inst, ml) =>
+        {
+            if (inst.Meter.Name == meter.Name)
+            {
+                ml.EnableMeasurementEvents(inst);
+            }
+        };
+        listener.SetMeasurementEventCallback<long>((inst, value, tags, _) =>
+        {
+            tagSets.Add(tags.ToArray());
+        });
+        listener.Start();
+
         using var cache = new MeteredMemoryCache(inner, meter, options);
-        Assert.Equal(cacheName, cache.Name);
+
+        // Perform operation to emit metrics
+        cache.TryGetValue("key", out _);
+
+        // Record Observable instruments
+        listener.RecordObservableInstruments();
+
+        // Verify cache.name tag has the CORRECT value (from CacheName, not AdditionalTags)
+        Assert.NotEmpty(tagSets);
+        Assert.All(tagSets, tags =>
+        {
+            var cacheNameTags = tags.Where(t => t.Key == "cache.name").ToList();
+            // Should have exactly one cache.name tag
+            Assert.Single(cacheNameTags);
+            // The value should be the constructor-specified name, not the duplicate
+            Assert.Equal(cacheName, (string?)cacheNameTags[0].Value);
+        });
+
+        // Verify env tag IS present
+        Assert.Contains(tagSets, tags =>
+            tags.Any(t => t.Key == "env" && (string?)t.Value == "test"));
     }
 
     #endregion

tests/Unit/MeteredMemoryCacheSharedTests.cs

@@ -79,8 +79,8 @@ public void TryGet_WithNamedCache_RecordsMetricsWithCacheName()
         Assert.Equal(1, harness.GetAggregatedCount("cache.requests", hitTag));
         Assert.Equal(1, harness.GetAggregatedCount("cache.requests", missTag));
 
-        Assert.Contains(true, harness.AllMeasurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "tryget-cache")));
+        harness.AssertAllMeasurementsHaveTags("cache.requests",
+            new KeyValuePair<string, object?>("cache.name", "tryget-cache"));
     }
 
     /// <summary>
@@ -108,9 +108,8 @@ public void GetOrCreate_WithNamedCache_RecordsMetricsWithCacheName()
         Assert.Equal(1, harness.GetAggregatedCount("cache.requests", hitTag));
         Assert.Equal(1, harness.GetAggregatedCount("cache.requests", missTag));
 
-        // Verify cache.name tag is present
-        Assert.Contains(true, harness.AllMeasurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "getorcreate-cache")));
+        harness.AssertAllMeasurementsHaveTags("cache.requests",
+            new KeyValuePair<string, object?>("cache.name", "getorcreate-cache"));
     }
 
     /// <summary>
@@ -132,15 +131,13 @@ public void WithAdditionalTags_EmitsAllTagsOnMetrics()
         subject.Cache.Set("k", 100);
         subject.Cache.TryGetValue("k", out _); // hit
 
-        // Verify cache.name tag is present
-        Assert.Contains(true, harness.AllMeasurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "tagged-cache")));
+        harness.AssertAllMeasurementsHaveTags("cache.requests",
+            new KeyValuePair<string, object?>("cache.name", "tagged-cache"));
 
-        // Verify additional tags are present
-        Assert.Contains(true, harness.AllMeasurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "environment" && (string?)tag.Value == "test")));
-        Assert.Contains(true, harness.AllMeasurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "region" && (string?)tag.Value == "us-west-2")));
+        // Verify additional tags are present on all measurements
+        harness.AssertAllMeasurementsHaveTags("cache.requests",
+            new KeyValuePair<string, object?>("environment", "test"),
+            new KeyValuePair<string, object?>("region", "us-west-2"));
     }
 
     /// <summary>

tests/Unit/MeteredMemoryCacheTests.cs

@@ -129,6 +129,18 @@ public long GetAggregatedCount(string instrumentName, params KeyValuePair<string
             }
         }
 
+        /// <summary>
+        /// Asserts that ALL measurements have the specified tag with the expected value.
+        /// Fails if the measurement list is empty (vacuous truth guard).
+        /// </summary>
+        public void AssertAllMeasurementsHaveTag(string tagKey, string expectedValue)
+        {
+            var measurements = Measurements;
+            Assert.NotEmpty(measurements);
+            Assert.All(measurements, m =>
+                Assert.Contains(m.Tags, t => t.Key == tagKey && (string?)t.Value == expectedValue));
+        }
+
         public void Dispose() => _listener.Dispose();
     }
 
@@ -214,9 +226,8 @@ public void MeteredMemoryCache_WithCacheName_EmitsCacheNameTag()
         cache.Set("k", 42);
         cache.TryGetValue("k", out _); // hit
 
-        // At least one measurement should have the cache.name tag
-        Assert.Contains(true, listener.Measurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "test-cache-name")));
+        // ALL measurements must carry the cache.name tag
+        listener.AssertAllMeasurementsHaveTag("cache.name", "test-cache-name");
     }
 
     [Fact]
@@ -860,15 +871,10 @@ public void OptionsConstructor_WithAdditionalTags_EmitsAllTagsOnMetrics()
         cache.Set("k", 100);
         cache.TryGetValue("k", out _); // hit
 
-        // Verify cache.name tag is present
-        Assert.Contains(true, listener.Measurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "tagged-cache")));
-
-        // Verify additional tags are present
-        Assert.Contains(true, listener.Measurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "environment" && (string?)tag.Value == "test")));
-        Assert.Contains(true, listener.Measurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "region" && (string?)tag.Value == "us-west-2")));
+        // ALL measurements must carry each expected tag
+        listener.AssertAllMeasurementsHaveTag("cache.name", "tagged-cache");
+        listener.AssertAllMeasurementsHaveTag("environment", "test");
+        listener.AssertAllMeasurementsHaveTag("region", "us-west-2");
     }
 
     [Fact]
@@ -1025,8 +1031,7 @@ public void TryGet_WithNamedCache_RecordsMetricsWithCacheName()
         Assert.Equal(1, listener.GetAggregatedCount("cache.requests", hitTag));
         Assert.Equal(1, listener.GetAggregatedCount("cache.requests", missTag));
 
-        Assert.Contains(true, listener.Measurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "tryget-cache")));
+        listener.AssertAllMeasurementsHaveTag("cache.name", "tryget-cache");
     }
 
     [Fact]
@@ -1052,9 +1057,8 @@ public void GetOrCreate_WithNamedCache_RecordsMetricsWithCacheName()
         Assert.Equal(1, listener.GetAggregatedCount("cache.requests", hitTag));
         Assert.Equal(1, listener.GetAggregatedCount("cache.requests", missTag));
 
-        // Verify cache.name tag is present
-        Assert.Contains(true, listener.Measurements.Select(m =>
-            m.Tags.Any(tag => tag.Key == "cache.name" && (string?)tag.Value == "getorcreate-cache")));
+        // ALL measurements must carry cache.name tag
+        listener.AssertAllMeasurementsHaveTag("cache.name", "getorcreate-cache");
     }
 
     [Fact]