Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Commit 5f9a0fa

Browse files
authored
Fix flaky LinkTest (#2082)
LinkTest has recently been having flaky failures. It turns out that the `link_ToString()` test relied on hash ordering. The failing assertion only passed when a copy of the `HashMap` had the same `toString()` as the original map. If copying the map changed the iteration order, the test would fail. This entire test function is questionable, however, as it is testing the `toString()` method on an `AutoValue`. This is problematic, because `AutoValue` specifies its `toString()` as "returning a useful (but unspecified) string representation of the instance" (see https://github.com/google/auto/blob/master/value/userguide/index.md#whats-going-on-here). As such, it could make sense to remove this test altogether. This change does not remove the test, because it's operating under the assumption that the test is wanted. This change fixes the test by checking that every entry in the map is contained in the `link.toString()`. This means that the order of the entries in the `link.toString()` is no longer important. Note that if `Link` is changed to use a different map implementation not based on `AbstractMap` that does not similarly guarantee its `toString()`, this test may break, although the previous assert would also break in such a situation. Tested: - I tested this change by running LinkTest 100 times to verify that the flaky failure went away.
1 parent 4f217b4 commit 5f9a0fa

File tree

1 file changed

+9
-2
lines changed

1 file changed

+9
-2
lines changed

api/src/test/java/io/opencensus/trace/LinkTest.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,18 @@ public void link_ToString() {
102102
assertThat(link.toString()).contains(spanContext.getTraceId().toString());
103103
assertThat(link.toString()).contains(spanContext.getSpanId().toString());
104104
assertThat(link.toString()).contains("CHILD_LINKED_SPAN");
105-
assertThat(link.toString()).contains(attributesMap.toString());
105+
for (Map.Entry<String, AttributeValue> entry : attributesMap.entrySet()) {
106+
// This depends on HashMap#toString(), via AbstractMap#toString(), having a specified format.
107+
// In particular, each entry is formatted as `key=value`, with no spaces around the `=`.
108+
// If Link is changed to use something other than a HashMap, this may no longer pass.
109+
assertThat(link.toString()).contains(entry.getKey() + "=" + entry.getValue());
110+
}
106111
link = Link.fromSpanContext(spanContext, Type.PARENT_LINKED_SPAN, attributesMap);
107112
assertThat(link.toString()).contains(spanContext.getTraceId().toString());
108113
assertThat(link.toString()).contains(spanContext.getSpanId().toString());
109114
assertThat(link.toString()).contains("PARENT_LINKED_SPAN");
110-
assertThat(link.toString()).contains(attributesMap.toString());
115+
for (Map.Entry<String, AttributeValue> entry : attributesMap.entrySet()) {
116+
assertThat(link.toString()).contains(entry.getKey() + "=" + entry.getValue());
117+
}
111118
}
112119
}

0 commit comments

Comments
 (0)