pskevin commented on a change in pull request #12071:
URL: https://github.com/apache/beam/pull/12071#discussion_r461839228
##########
File path:
runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ValidateRunnerXlangTest.java
##########
@@ -54,15 +54,36 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/** Test External transforms. */
+/**
+ * Runner Validation Test Suite for Cross-language Transforms.
+ *
+ * <p>As per Beams's Portability Framework design, Cross-language transforms
should work out of the
+ * box. In spite of this, there always exists a possibility of rough edges
existing. It could be
+ * caused due to unpolished implementation of any part of the execution code
path, for example: –>
+ * Transform expansion [SDK] –> Pipeline construction [SDK] –> Cross-language
artifact staging
Review comment:
Oh wow. Technically, "–> Transform expansion [SDK]" is supposed to be
one bullet point (and applies to others as well). Seems like spotless didn't
like it and put them as you see it. I'll change it to commas.
##########
File path:
runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ValidateRunnerXlangTest.java
##########
@@ -54,15 +54,36 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/** Test External transforms. */
+/**
+ * Runner Validation Test Suite for Cross-language Transforms.
+ *
+ * <p>As per Beams's Portability Framework design, Cross-language transforms
should work out of the
+ * box. In spite of this, there always exists a possibility of rough edges
existing. It could be
+ * caused due to unpolished implementation of any part of the execution code
path, for example: –>
+ * Transform expansion [SDK] –> Pipeline construction [SDK] –> Cross-language
artifact staging
+ * [Runner] –> Language specific serialization/deserialization of PCollection
(and other data types)
+ * [Runner/SDK]
+ *
+ * <p>In an effort to improve developer visibility into potential problems,
this test suite
+ * validates correct execution of 5 Core Beam transforms when used as
cross-language transforms
+ * within the Java SDK from any foreign SDK: –> ParDo
+ * (https://beam.apache.org/documentation/programming-guide/#pardo) –>
GroupByKey
Review comment:
Ditto as my previous comment. None of this is organized as I intended it
originally. Either spotless or lint has removed all new lines because none of
the text is separated as markdown (when compared to the previous version).
My guess with the link is that since it's presence of the same line would
exceed the allowed number of characters per line, it was just wrapped as you
see it.
##########
File path:
runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ValidateRunnerXlangTest.java
##########
@@ -110,6 +131,14 @@ private void waitForReady() {
}
}
+ /**
+ * Motivation behind singleInputOutputTest.
+ *
+ * <p>Target transform – ParDo
(https://beam.apache.org/documentation/programming-guide/#pardo)
+ * Test scenario – Mapping elements from a single input collection to a
single output collection
+ * Boundary conditions checked – –> PCollection<?> to external transforms –>
PCollection<?> from
Review comment:
Ditto as my comment earlier.
##########
File path:
runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ValidateRunnerXlangTest.java
##########
@@ -110,6 +131,14 @@ private void waitForReady() {
}
}
+ /**
+ * Motivation behind singleInputOutputTest.
+ *
+ * <p>Target transform – ParDo
(https://beam.apache.org/documentation/programming-guide/#pardo)
+ * Test scenario – Mapping elements from a single input collection to a
single output collection
+ * Boundary conditions checked – –> PCollection<?> to external transforms –>
PCollection<?> from
+ * external transforms
Review comment:
Thanks for catching that.
##########
File path:
runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ValidateRunnerXlangTest.java
##########
@@ -135,6 +172,15 @@ public void multiInputOutputWithSideInputTest() {
PAssert.that(pTuple.get("side")).containsInAnyOrder("ss");
}
+ /**
+ * Motivation behind groupByKeyTest.
+ *
+ * <p>Target transform – GroupByKey
+ * (https://beam.apache.org/documentation/programming-guide/#groupbykey)
Test scenario – Grouping
Review comment:
Ditto as my comment earlier. I'll see how to resolve this.
##########
File path: sdks/python/apache_beam/transforms/validate_runner_xlang_test.py
##########
@@ -14,6 +14,42 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
+
+"""
+###########################################################
+Runner Validation Test Suite for Cross-language Transforms
+###########################################################
+ As per Beams's Portability Framework design, Cross-language transforms
+ should work out of the box. In spite of this, there always exists a
+ possibility of rough edges existing. It could be caused due to unpolished
+ implementation of any part of the execution code path, for example:
+ - Transform expansion [SDK]
+ - Pipeline construction [SDK]
+ - Cross-language artifact staging [Runner]
+ - Language specific serialization/deserialization of PCollection (and
+ other data types) [Runner/SDK]
+
+ In an effort to improve developer visibility into potential problems,
+ this test suite validates correct execution of 5 Core Beam transforms when
+ used as cross-language transforms within the Python SDK from any foreign SDK:
+ - ParDo
+ (https://beam.apache.org/documentation/programming-guide/#pardo)
+ - GroupByKey
+ (https://beam.apache.org/documentation/programming-guide/#groupbykey)
+ - CoGroupByKey
+ (https://beam.apache.org/documentation/programming-guide/#cogroupbykey)
+ - Combine
Review comment:
Thanks. :)
##########
File path: sdks/python/apache_beam/transforms/validate_runner_xlang_test.py
##########
@@ -57,6 +102,15 @@ def run_prefix(self, pipeline):
assert_that(res, equal_to(['0a', '0b']))
def run_multi_input_output_with_sideinput(self, pipeline):
+ """
+ Target transform - ParDo
+ (https://beam.apache.org/documentation/programming-guide/#pardo)
+ Test scenario - Mapping elements from multiple input collections (main
+ and side) to multiple output collections (main and side)
Review comment:
Thanks for catching that. Will make the necessary change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
