Copilot commented on code in PR #6221:
URL: https://github.com/apache/ignite-3/pull/6221#discussion_r2192966380
##########
modules/platforms/dotnet/Apache.Ignite.Tests/MetricsTests.cs:
##########
@@ -397,22 +416,13 @@ public void AssertMetric(string name, int value, int
timeoutMs = 1000)
messageFactory: () => $"{name}: expected '{value}', but was
'{GetMetric(name)}'");
}
- public void AssertTaggedMetric(string name, int value, string
nodeAddr, Guid? clientId)
+ public void AssertTaggedMetric(string name, int value, string
nodeAddr, Guid? clientId, int timeoutMs = 1000)
{
- if (clientId == null)
- {
- // Client id is not known, find by name and node address.
- var val = _metricsWithTags.Single(x =>
- x.Key.StartsWith($"{name}_{MetricTags.ClientId}=",
StringComparison.Ordinal) &&
- x.Key.EndsWith($",{MetricTags.NodeAddress}={nodeAddr}",
StringComparison.Ordinal));
-
- Assert.AreEqual(value, val.Value);
- }
- else
- {
- var taggedName =
$"{name}_{MetricTags.ClientId}={clientId},{MetricTags.NodeAddress}={nodeAddr}";
- Assert.AreEqual(value, _metricsWithTags[taggedName]);
- }
+ TestUtils.WaitForCondition(
+ condition: () => GetTaggedMetric(name, nodeAddr, clientId) ==
value,
+ timeoutMs: timeoutMs,
+ messageFactory: () => $"{name} for {nodeAddr} ({clientId}):
expected '{value}', " +
+ $"but was '{GetTaggedMetric(name,
nodeAddr, clientId)}'");
Review Comment:
[nitpick] Avoid calling `GetTaggedMetric` twice (in the condition and the
message). Capture its result in a local variable and reuse it to ensure
consistency and reduce duplicate work.
```suggestion
int taggedMetric = GetTaggedMetric(name, nodeAddr, clientId);
TestUtils.WaitForCondition(
condition: () => taggedMetric == value,
timeoutMs: timeoutMs,
messageFactory: () => $"{name} for {nodeAddr} ({clientId}):
expected '{value}', " +
$"but was '{taggedMetric}'");
```
##########
modules/platforms/dotnet/Apache.Ignite.Tests/MetricsTests.cs:
##########
@@ -389,6 +389,25 @@ public int GetMetric(string name)
return _metrics.TryGetValue(name, out var val) ? (int)val : 0;
}
+ public int GetTaggedMetric(string name, string nodeAddr, Guid?
clientId)
+ {
+ _listener.RecordObservableInstruments();
+
+ if (clientId != null)
+ {
+ var taggedName =
$"{name}_{MetricTags.ClientId}={clientId},{MetricTags.NodeAddress}={nodeAddr}";
+ return _metricsWithTags.TryGetValue(taggedName, out var val) ?
(int)val : 0;
+ }
+
+ // Client id is not known, find by name and node address.
+ return _metricsWithTags
+ .Where(x =>
+ x.Key.StartsWith($"{name}_{MetricTags.ClientId}=",
StringComparison.Ordinal) &&
+ x.Key.EndsWith($",{MetricTags.NodeAddress}={nodeAddr}",
StringComparison.Ordinal))
+ .Select(x => (int)x.Value)
+ .SingleOrDefault();
Review Comment:
Using `SingleOrDefault` will return 0 when the expected tagged metric is
missing, potentially masking issues. Consider using `Single()` to throw an
exception if the metric isn't found.
```suggestion
.Single();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
