You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:28:04 UTC
[GitHub] [beam] chamikaramj commented on a change in pull request #12071: [BEAM-9932] Add documentation describing cross-language test pipelines
chamikaramj commented on a change in pull request #12071:
URL: https://github.com/apache/beam/pull/12071#discussion_r461161707
##########
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:
Does "->" have a special meaning ? If not we should just use comma here.
##########
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 regarding ->
Also please move the link to the same line as the corresponding transform for easy readability of anyone who is just reading code.
##########
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:
Move "Test scenario" to a new line here and below.
##########
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 regarding using comma instead of "->" here and below.
##########
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:
This reads much better :)
##########
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:
Missing full stop here and below.
##########
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:
Missing full stop here and below.
----------------------------------------------------------------
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:
users@infra.apache.org