From a52c1b6cdc033f64f526cff9cf0a2c4d8f23aa14 Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Mon, 28 Mar 2016 13:38:35 -0700 Subject: [PATCH 1/2] DisplayData tweaks based on transform usage. * Add boolean-valued display data. * Implement equality for DislpayData.Item * Add ability to override namespace for included subcomponents. * Additional Matchers for testing display data * Update DisplayData inner class privacy --- .../sdk/transforms/display/DisplayData.java | 85 ++++++-- .../display/DisplayDataMatchers.java | 181 +++++++++++++++++- .../display/DisplayDataMatchersTest.java | 63 +++++- .../transforms/display/DisplayDataTest.java | 128 ++++++++----- 4 files changed, 391 insertions(+), 66 deletions(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index dadc7309da31..a697cb342b94 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -87,7 +87,7 @@ public Map asMap() { public String toString() { StringBuilder builder = new StringBuilder(); boolean isFirstLine = true; - for (Map.Entry entry : entries.entrySet()) { + for (Item entry : entries.values()) { if (isFirstLine) { isFirstLine = false; } else { @@ -106,13 +106,18 @@ public String toString() { */ public interface Builder { /** - * Include display metadata from the specified subcomponent. For example, a {@link ParDo} + * Register display metadata from the specified subcomponent. For example, a {@link ParDo} * transform includes display metadata from the encapsulated {@link DoFn}. - * - * @return A builder instance to continue to build in a fluent-style. */ Builder include(HasDisplayData subComponent); + /** + * Register display metadata from the specified subcomponent, using the specified namespace. + * For example, a {@link ParDo} transform includes display metadata from the encapsulated + * {@link DoFn}. + */ + Builder include(HasDisplayData subComponent, Class namespace); + /** * Register the given string display metadata. The metadata item will be registered with type * {@link DisplayData.Type#STRING}, and is identified by the specified key and namespace from @@ -134,6 +139,13 @@ public interface Builder { */ ItemBuilder add(String key, double value); + /** + * Register the given floating point display metadata. The metadata item will be registered with + * type {@link DisplayData.Type#BOOLEAN}, and is identified by the specified key and namespace + * from the current transform or component. + */ + ItemBuilder add(String key, boolean value); + /** * Register the given timestamp display metadata. The metadata item will be registered with type * {@link DisplayData.Type#TIMESTAMP}, and is identified by the specified key and namespace from @@ -286,7 +298,35 @@ public String getLinkUrl() { @Override public String toString() { - return getValue(); + return String.format("%s:%s=%s", ns, key, value); + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof Item) { + Item that = (Item) obj; + return Objects.equals(this.ns, that.ns) + && Objects.equals(this.key, that.key) + && Objects.equals(this.type, that.type) + && Objects.equals(this.value, that.value) + && Objects.equals(this.shortValue, that.shortValue) + && Objects.equals(this.label, that.label) + && Objects.equals(this.url, that.url); + } + + return false; + } + + @Override + public int hashCode() { + return Objects.hash( + this.ns, + this.key, + this.type, + this.value, + this.shortValue, + this.label, + this.url); } private Item withLabel(String label) { @@ -312,8 +352,12 @@ public static class Identifier { private final String ns; private final String key; - static Identifier of(Class namespace, String key) { - return new Identifier(namespace.getName(), key); + public static Identifier of(Class namespace, String key) { + return of(namespace.getName(), key); + } + + public static Identifier of(String namespace, String key) { + return new Identifier(namespace, key); } private Identifier(String ns, String key) { @@ -354,7 +398,7 @@ public String toString() { /** * Display metadata type. */ - enum Type { + public enum Type { STRING { @Override FormattedItemValue format(Object value) { @@ -373,6 +417,12 @@ FormattedItemValue format(Object value) { return new FormattedItemValue(Double.toString((Double) value)); } }, + BOOLEAN() { + @Override + FormattedItemValue format(Object value) { + return new FormattedItemValue(Boolean.toString((boolean) value)); + } + }, TIMESTAMP() { @Override FormattedItemValue format(Object value) { @@ -402,7 +452,7 @@ FormattedItemValue format(Object value) { abstract FormattedItemValue format(Object value); } - private static class FormattedItemValue { + static class FormattedItemValue { private final String shortValue; private final String longValue; @@ -415,11 +465,11 @@ private FormattedItemValue(String longValue, String shortValue) { this.shortValue = shortValue; } - private String getLongValue () { + String getLongValue () { return this.longValue; } - private String getShortValue() { + String getShortValue() { return this.shortValue; } } @@ -445,11 +495,17 @@ private static InternalBuilder forRoot(HasDisplayData instance) { @Override public Builder include(HasDisplayData subComponent) { + checkNotNull(subComponent); + return include(subComponent, subComponent.getClass()); + } + + @Override + public Builder include(HasDisplayData subComponent, Class namespace) { checkNotNull(subComponent); boolean newComponent = visited.add(subComponent); if (newComponent) { Class prevNs = this.latestNs; - this.latestNs = subComponent.getClass(); + this.latestNs = namespace; subComponent.populateDisplayData(this); this.latestNs = prevNs; } @@ -473,6 +529,11 @@ public ItemBuilder add(String key, double value) { return addItem(key, Type.FLOAT, value); } + @Override + public ItemBuilder add(String key, boolean value) { + return addItem(key, Type.BOOLEAN, value); + } + @Override public ItemBuilder add(String key, Instant value) { checkNotNull(value); diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java index 2753aafa7bb3..89d183452266 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchers.java @@ -16,13 +16,19 @@ package com.google.cloud.dataflow.sdk.transforms.display; +import static org.hamcrest.Matchers.allOf; + import com.google.cloud.dataflow.sdk.transforms.display.DisplayData.Item; +import com.google.common.collect.Sets; +import org.hamcrest.CustomTypeSafeMatcher; import org.hamcrest.Description; import org.hamcrest.FeatureMatcher; import org.hamcrest.Matcher; import org.hamcrest.Matchers; import org.hamcrest.TypeSafeDiagnosingMatcher; +import org.joda.time.Duration; +import org.joda.time.Instant; import java.util.Collection; @@ -42,6 +48,71 @@ public static Matcher hasDisplayItem() { return hasDisplayItem(Matchers.any(DisplayData.Item.class)); } + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and String value. + */ + public static Matcher hasDisplayItem(String key, String value) { + return hasDisplayItem(key, DisplayData.Type.STRING, value); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and Boolean value. + */ + public static Matcher hasDisplayItem(String key, Boolean value) { + return hasDisplayItem(key, DisplayData.Type.BOOLEAN, value); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and Duration value. + */ + public static Matcher hasDisplayItem(String key, Duration value) { + return hasDisplayItem(key, DisplayData.Type.DURATION, value); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and Float value. + */ + public static Matcher hasDisplayItem(String key, double value) { + return hasDisplayItem(key, DisplayData.Type.FLOAT, value); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and Integer value. + */ + public static Matcher hasDisplayItem(String key, long value) { + return hasDisplayItem(key, DisplayData.Type.INTEGER, value); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and Class value. + */ + public static Matcher hasDisplayItem(String key, Class value) { + return hasDisplayItem(key, DisplayData.Type.JAVA_CLASS, value); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains an item with the + * specified key and Timestamp value. + */ + public static Matcher hasDisplayItem(String key, Instant value) { + return hasDisplayItem(key, DisplayData.Type.TIMESTAMP, value); + } + + private static Matcher hasDisplayItem( + String key, DisplayData.Type type, Object value) { + DisplayData.FormattedItemValue formattedValue = type.format(value); + return hasDisplayItem(allOf( + hasKey(key), + hasType(type), + hasValue(formattedValue.getLongValue()))); + } + /** * Creates a matcher that matches if the examined {@link DisplayData} contains any item * matching the specified {@code itemMatcher}. @@ -68,13 +139,93 @@ protected boolean matchesSafely(DisplayData data, Description mismatchDescriptio Collection items = data.items(); boolean isMatch = Matchers.hasItem(itemMatcher).matches(items); if (!isMatch) { - mismatchDescription.appendText("found " + items.size() + " non-matching items"); + mismatchDescription.appendText("found " + items.size() + " non-matching item(s):\n"); + mismatchDescription.appendValue(data); } return isMatch; } } + /** + * Create a matcher that matches if the examined {@link DisplayData} contains all display data + * registered from the specified subcomponent. + */ + public static Matcher includes(final HasDisplayData subComponent) { + return includes(subComponent, subComponent.getClass()); + } + + /** + * Create a matcher that matches if the examined {@link DisplayData} contains all display data + * registered from the specified subcomponent and namespace. + */ + public static Matcher includes( + final HasDisplayData subComponent, final Class namespace) { + return new CustomTypeSafeMatcher("includes subcomponent") { + @Override + protected boolean matchesSafely(DisplayData displayData) { + DisplayData subComponentData = DisplayData.from(subComponent); + if (subComponentData.items().size() == 0) { + throw new UnsupportedOperationException("subComponent contains no display data; " + + "cannot verify whether it is included"); + } + + DisplayDataComparision comparison = checkSubset(displayData, subComponentData, namespace); + return comparison.missingItems.isEmpty(); + } + + + @Override + protected void describeMismatchSafely( + DisplayData displayData, Description mismatchDescription) { + DisplayData subComponentDisplayData = DisplayData.from(subComponent); + DisplayDataComparision comparison = checkSubset( + displayData, subComponentDisplayData, subComponent.getClass()); + + mismatchDescription + .appendText("did not include:\n") + .appendValue(comparison.missingItems) + .appendText("\nNon-matching items:\n") + .appendValue(comparison.unmatchedItems); + } + + private DisplayDataComparision checkSubset( + DisplayData displayData, DisplayData included, Class namespace) { + DisplayDataComparision comparison = new DisplayDataComparision(displayData.items()); + for (Item item : included.items()) { + Item matchedItem = displayData.asMap().get( + DisplayData.Identifier.of(namespace, item.getKey())); + + if (matchedItem != null) { + comparison.matched(matchedItem); + } else { + comparison.missing(item); + } + } + + return comparison; + } + + class DisplayDataComparision { + Collection missingItems; + Collection unmatchedItems; + + DisplayDataComparision(Collection superset) { + missingItems = Sets.newHashSet(); + unmatchedItems = Sets.newHashSet(superset); + } + + void matched(Item supersetItem) { + unmatchedItems.remove(supersetItem); + } + + void missing(Item subsetItem) { + missingItems.add(subsetItem); + } + } + }; + } + /** * Creates a matcher that matches if the examined {@link DisplayData.Item} contains a key * with the specified value. @@ -95,4 +246,32 @@ protected String featureValueOf(DisplayData.Item actual) { } }; } + + public static Matcher hasType(DisplayData.Type type) { + return hasType(Matchers.is(type)); + } + + public static Matcher hasType(Matcher typeMatcher) { + return new FeatureMatcher( + typeMatcher, "with type", "type") { + @Override + protected DisplayData.Type featureValueOf(DisplayData.Item actual) { + return actual.getType(); + } + }; + } + + public static Matcher hasValue(String value) { + return hasValue(Matchers.is(value)); + } + + public static Matcher hasValue(Matcher valueMatcher) { + return new FeatureMatcher( + valueMatcher, "with value", "value") { + @Override + protected String featureValueOf(DisplayData.Item actual) { + return actual.getValue(); + } + }; + } } diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java index 2636cf85c840..595d519e6840 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataMatchersTest.java @@ -18,11 +18,13 @@ import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasDisplayItem; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasKey; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasType; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasValue; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.includes; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.core.StringStartsWith.startsWith; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; import com.google.cloud.dataflow.sdk.transforms.PTransform; import com.google.cloud.dataflow.sdk.transforms.display.DisplayData.Builder; @@ -45,7 +47,7 @@ public void testHasDisplayItem() { Matcher matcher = hasDisplayItem(); assertFalse(matcher.matches(DisplayData.none())); - assertTrue(matcher.matches(createDisplayDataWithItem("foo", "bar"))); + assertThat(createDisplayDataWithItem("foo", "bar"), matcher); } @Test @@ -58,15 +60,68 @@ public void testHasDisplayItemDescription() { matcher.describeMismatch(DisplayData.none(), mismatchDesc); assertThat(desc.toString(), startsWith("display data with item: ")); - assertThat(mismatchDesc.toString(), containsString("found 0 non-matching items")); + assertThat(mismatchDesc.toString(), containsString("found 0 non-matching item(s)")); } @Test public void testHasKey() { Matcher matcher = hasDisplayItem(hasKey("foo")); - assertTrue(matcher.matches(createDisplayDataWithItem("foo", "bar"))); assertFalse(matcher.matches(createDisplayDataWithItem("fooz", "bar"))); + + assertThat(createDisplayDataWithItem("foo", "bar"), matcher); + } + + @Test + public void testHasType() { + Matcher matcher = hasDisplayItem(hasType(DisplayData.Type.JAVA_CLASS)); + + DisplayData data = DisplayData.from(new PTransform, PCollection>() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("foo", DisplayDataMatchersTest.class); + } + }); + + assertFalse(matcher.matches(createDisplayDataWithItem("fooz", "bar"))); + assertThat(data, matcher); + } + + @Test + public void testHasValue() { + Matcher matcher = hasDisplayItem(hasValue("bar")); + + assertFalse(matcher.matches(createDisplayDataWithItem("foo", "baz"))); + assertThat(createDisplayDataWithItem("foo", "bar"), matcher); + } + + @Test + public void testIncludes() { + final HasDisplayData subComponent = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("foo", "bar"); + } + }; + HasDisplayData hasSubcomponent = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder + .include(subComponent) + .add("foo2", "bar2"); + } + }; + HasDisplayData sameKeyDifferentNamespace = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("foo", "bar"); + } + }; + Matcher matcher = includes(subComponent); + + assertFalse(matcher.matches(DisplayData.from(sameKeyDifferentNamespace))); + assertThat(DisplayData.from(hasSubcomponent), matcher); + assertThat(DisplayData.from(subComponent), matcher); } private DisplayData createDisplayDataWithItem(final String key, final String value) { diff --git a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java index dfc8c38f52fb..47d6e7806230 100644 --- a/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java +++ b/sdks/java/core/src/test/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayDataTest.java @@ -18,6 +18,9 @@ import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasDisplayItem; import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasKey; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasType; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.hasValue; +import static com.google.cloud.dataflow.sdk.transforms.display.DisplayDataMatchers.includes; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.everyItem; @@ -91,7 +94,7 @@ public void populateDisplayData(DisplayData.Builder builder) { .include(subComponent2) .add("MinSproggles", 200) .withLabel("Mimimum Required Sproggles") - .add("LazerOrientation", "NORTH") + .add("FireLazers", true) .add("TimeBomb", Instant.now().plus(Duration.standardDays(1))) .add("FilterLogic", subComponent1.getClass()) .add("ServiceUrl", "google.com/fizzbang") @@ -126,12 +129,12 @@ public void testCanBuild() { DisplayData.from(new HasDisplayData() { @Override public void populateDisplayData(DisplayData.Builder builder) { - builder.add("Foo", "bar"); + builder.add("foo", "bar"); } }); assertThat(data.items(), hasSize(1)); - assertThat(data, hasDisplayItem(hasKey("Foo"))); + assertThat(data, hasDisplayItem("foo", "bar")); } @Test @@ -147,7 +150,7 @@ public void populateDisplayData(DisplayData.Builder builder) { Map map = data.asMap(); assertEquals(map.size(), 1); - assertThat(data, hasDisplayItem(hasKey("foo"))); + assertThat(data, hasDisplayItem("foo", "bar")); assertEquals(map.values(), data.items()); } @@ -163,8 +166,8 @@ public void testItemProperties() { allOf( hasNamespace(Matchers.>is(ConcreteComponent.class)), hasKey("now"), - hasType(is(DisplayData.Type.TIMESTAMP)), - hasValue(is(ISO_FORMATTER.print(value))), + hasType(DisplayData.Type.TIMESTAMP), + hasValue(ISO_FORMATTER.print(value)), hasShortValue(nullValue(String.class)), hasLabel(is("the current instant")), hasUrl(is("http://time.gov")))); @@ -218,14 +221,35 @@ public void populateDisplayData(DisplayData.Builder builder) { } }); - assertThat( - data, - hasDisplayItem( - allOf( - hasKey("foo"), - hasNamespace(Matchers.>is(subComponent.getClass()))))); + assertThat(data, includes(subComponent)); + } + + @Test + public void testIncludesNamespaceOverride() { + final HasDisplayData subComponent = new HasDisplayData() { + @Override + public void populateDisplayData(DisplayData.Builder builder) { + builder.add("foo", "bar"); + } + }; + + final HasDisplayData namespaceOverride = new HasDisplayData(){ + @Override + public void populateDisplayData(Builder builder) { + } + }; + + DisplayData data = DisplayData.from(new HasDisplayData() { + @Override + public void populateDisplayData(DisplayData.Builder builder) { + builder.include(subComponent, namespaceOverride.getClass()); + } + }); + + assertThat(data, includes(subComponent, namespaceOverride.getClass())); } + @Test public void testIdentifierEquality() { new EqualsTester() @@ -237,6 +261,33 @@ public void testIdentifierEquality() { .testEquals(); } + @Test + public void testItemEquality() { + HasDisplayData component1 = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("foo", "bar"); + } + }; + HasDisplayData component2 = new HasDisplayData() { + @Override + public void populateDisplayData(Builder builder) { + builder.add("foo", "bar"); + } + }; + + DisplayData component1DisplayData1 = DisplayData.from(component1); + DisplayData component1DisplayData2 = DisplayData.from(component1); + DisplayData component2DisplayData = DisplayData.from(component2); + + new EqualsTester() + .addEqualityGroup( + component1DisplayData1.items().toArray()[0], + component1DisplayData2.items().toArray()[0]) + .addEqualityGroup(component2DisplayData.items().toArray()[0]) + .testEquals(); + } + @Test public void testAnonymousClassNamespace() { DisplayData data = @@ -403,6 +454,7 @@ public void populateDisplayData(DisplayData.Builder builder) { .add("string", "foobar") .add("integer", 123) .add("float", 3.14) + .add("boolean", true) .add("java_class", DisplayDataTest.class) .add("timestamp", Instant.now()) .add("duration", Duration.standardHours(1)); @@ -411,18 +463,19 @@ public void populateDisplayData(DisplayData.Builder builder) { Collection items = data.items(); assertThat( - items, hasItem(allOf(hasKey("string"), hasType(is(DisplayData.Type.STRING))))); + items, hasItem(allOf(hasKey("string"), hasType(DisplayData.Type.STRING)))); assertThat( - items, hasItem(allOf(hasKey("integer"), hasType(is(DisplayData.Type.INTEGER))))); - assertThat(items, hasItem(allOf(hasKey("float"), hasType(is(DisplayData.Type.FLOAT))))); + items, hasItem(allOf(hasKey("integer"), hasType(DisplayData.Type.INTEGER)))); + assertThat(items, hasItem(allOf(hasKey("float"), hasType(DisplayData.Type.FLOAT)))); + assertThat(items, hasItem(allOf(hasKey("boolean"), hasType(DisplayData.Type.BOOLEAN)))); assertThat( items, - hasItem(allOf(hasKey("java_class"), hasType(is(DisplayData.Type.JAVA_CLASS))))); + hasItem(allOf(hasKey("java_class"), hasType(DisplayData.Type.JAVA_CLASS)))); assertThat( items, - hasItem(allOf(hasKey("timestamp"), hasType(is(DisplayData.Type.TIMESTAMP))))); + hasItem(allOf(hasKey("timestamp"), hasType(DisplayData.Type.TIMESTAMP)))); assertThat( - items, hasItem(allOf(hasKey("duration"), hasType(is(DisplayData.Type.DURATION))))); + items, hasItem(allOf(hasKey("duration"), hasType(DisplayData.Type.DURATION)))); } @Test @@ -437,6 +490,7 @@ public void populateDisplayData(DisplayData.Builder builder) { .add("string", "foobar") .add("integer", 123) .add("float", 3.14) + .add("boolean", true) .add("java_class", DisplayDataTest.class) .add("timestamp", now) .add("duration", oneHour); @@ -444,17 +498,13 @@ public void populateDisplayData(DisplayData.Builder builder) { }; DisplayData data = DisplayData.from(component); - Collection items = data.items(); - assertThat(items, hasItem(allOf(hasKey("string"), hasValue(is("foobar"))))); - assertThat(items, hasItem(allOf(hasKey("integer"), hasValue(is("123"))))); - assertThat(items, hasItem(allOf(hasKey("float"), hasValue(is("3.14"))))); - assertThat(items, hasItem(allOf(hasKey("java_class"), - hasValue(is(DisplayDataTest.class.getName())), - hasShortValue(is(DisplayDataTest.class.getSimpleName()))))); - assertThat(items, hasItem(allOf(hasKey("timestamp"), - hasValue(is(ISO_FORMATTER.print(now)))))); - assertThat(items, hasItem(allOf(hasKey("duration"), - hasValue(is(Long.toString(oneHour.getMillis())))))); + assertThat(data, hasDisplayItem("string", "foobar")); + assertThat(data, hasDisplayItem("integer", 123)); + assertThat(data, hasDisplayItem("float", 3.14)); + assertThat(data, hasDisplayItem("boolean", true)); + assertThat(data, hasDisplayItem("java_class", DisplayDataTest.class)); + assertThat(data, hasDisplayItem("timestamp", now)); + assertThat(data, hasDisplayItem("duration", oneHour)); } @Test @@ -581,16 +631,6 @@ protected Class featureValueOf(DisplayData.Item actual) { }; } - private static Matcher hasType(Matcher typeMatcher) { - return new FeatureMatcher( - typeMatcher, "display item with type", "type") { - @Override - protected DisplayData.Type featureValueOf(DisplayData.Item actual) { - return actual.getType(); - } - }; - } - private static Matcher hasLabel(Matcher labelMatcher) { return new FeatureMatcher( labelMatcher, "display item with label", "label") { @@ -611,16 +651,6 @@ protected String featureValueOf(DisplayData.Item actual) { }; } - private static Matcher hasValue(Matcher valueMatcher) { - return new FeatureMatcher( - valueMatcher, "display item with value", "value") { - @Override - protected String featureValueOf(DisplayData.Item actual) { - return actual.getValue(); - } - }; - } - private static Matcher hasShortValue(Matcher valueStringMatcher) { return new FeatureMatcher( valueStringMatcher, "display item with short value", "short value") { From ce5800842f83dcfce20b9149ede77dd5d468c46a Mon Sep 17 00:00:00 2001 From: Scott Wegner Date: Mon, 4 Apr 2016 16:31:09 -0700 Subject: [PATCH 2/2] fixup: review feedback --- .../cloud/dataflow/sdk/transforms/display/DisplayData.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java index a697cb342b94..54bc82b428e9 100644 --- a/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java +++ b/sdks/java/core/src/main/java/com/google/cloud/dataflow/sdk/transforms/display/DisplayData.java @@ -465,7 +465,7 @@ private FormattedItemValue(String longValue, String shortValue) { this.shortValue = shortValue; } - String getLongValue () { + String getLongValue() { return this.longValue; }