[TEST] [DELTA] Refine CI and add test detla 1.0.0 with Spark 3.1 #643

pan3793 · 2021-05-25T02:35:51Z

Why are the changes needed?

Fix -Pkyuubi-sql-spark_3.1 introduced by KYUUBI Add kyuubi-spark-extensions module #631 in GH action
Tune GH action and travis scripts style
Enable tests of detla 1.0.0 against Spark 3.1

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

pan3793 · 2021-05-25T02:36:32Z

pom.xml

+                        <groupId>*</groupId>
+                        <artifactId>*</artifactId>
+                    </exclusion>
+                </exclusions>


delta-io/delta#676

It's better to add some comments at this code place or add a todo to remind update this in future ?

codecov-commenter · 2021-05-25T03:36:56Z

Codecov Report

Merging #643 (e15a3ff) into master (10d72e0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #643   +/-   ##
=======================================
  Coverage   79.87%   79.87%           
=======================================
  Files         120      120           
  Lines        4671     4671           
  Branches      562      562           
=======================================
  Hits         3731     3731           
  Misses        624      624           
  Partials      316      316

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10d72e0...e15a3ff. Read the comment docs.

.github/workflows/master.yml

pan3793 · 2021-05-25T05:24:46Z

Travis failed with

The job exceeded the maximum log length, and has been terminated.

ulysses-you

thank you @pan3793 for driving this.

ulysses-you · 2021-05-25T05:35:26Z

.github/workflows/master.yml

-          - '-DwildcardSuites=org.apache.kyuubi.operation.tpcds.TPCDSOutputSchemaSuite,org.apache.kyuubi.operation.tpcds.TPCDSDDLSuite -Dmaven.plugin.scalatest.exclude.tags=""'
+          - '-Pspark-3.0 -Dspark.archive.mirror=https://archive.apache.org/dist/spark/spark-3.1.1 -Dspark.archive.name=spark-3.1.1-bin-hadoop2.7.tgz -Dmaven.plugin.scalatest.exclude.tags=org.apache.kyuubi.tags.ExtendedSQLTest,org.apache.kyuubi.tags.DeltaTest,org.apache.kyuubi.tags.IcebergTest'
+          - '-Pspark-3.1 -Pkyuubi-extension-spark_3.1'
+          - '-Pspark-3.1 -Pkyuubi-extension-spark_3.1 -Pspark-hadoop-3.2'


ah, seems we need to backport into branch-1.2.0

ulysses-you · 2021-05-25T05:36:22Z

kyuubi-common/src/test/java/org/apache/kyuubi/tags/DeltaTest.java

@@ -27,4 +27,4 @@
 @TagAnnotation
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.METHOD, ElementType.TYPE})
-public @interface DataLakeTest {}
+public @interface DeltaTest {}


Not a big issue, but shall we add a parent tag for DeltaTest and IcebergTest ?

I think we use tags here to mark tests which not support specific Spark versions, and due to the limitation of scalatest maven plugin, such as collection operation, the workflow scripts are not elegant now. Would you explain the benefit of adding a parent tag?

Not a strong opinion. I'm fine with both.

ulysses-you · 2021-05-25T05:38:17Z

pom.xml

+                        <groupId>*</groupId>
+                        <artifactId>*</artifactId>
+                    </exclusion>
+                </exclusions>


It's better to add some comments at this code place or add a todo to remind update this in future ?

ulysses-you · 2021-05-25T06:16:21Z

mistouch the close button : (

yaooqinn · 2021-05-25T06:31:29Z

.github/workflows/master.yml

      - name: Code coverage
        if: ${{ matrix.profiles == '' }}
        run: bash <(curl -s https://codecov.io/bash)
      - name: Detected Dependency List Change
-        if: ${{ ! contains(matrix.profiles, 'spark-3.2-snapshot') && ! contains(matrix.profiles, 'wildcardSuites') }}
+        if: ${{ ! contains(matrix.profiles, 'spark-3.2-snapshot') && ! contains(matrix.profiles, 'tpcds.tests=true') }}


instead of using matrix I prefer to move tpcds tests to another separate job like rat

And not related to this PR, we can mv spark-3.2-snapshot to a another yml with nightly build enabled?

yaooqinn · 2021-05-25T11:57:10Z

pom.xml

            </properties>
        </profile>

        <profile>
-            <id>spark-3.1</id>
+            <id>spark-3.2-snapshot</id>


spark-master？

sounds good

yaooqinn · 2021-05-25T12:04:52Z

.travis.yml

+  - |
+    if [[ "$JOB" == "tpcds-tests" ]]; then
+      mvn clean install $PROFILE -DskipTests -pl :kyuubi-spark-sql-engine,:kyuubi-common,:kyuubi-ha,:kyuubi-zookeeper
+      mvn test $PROFILE -Dtest=none -DwildcardSuites=org.apache.kyuubi.operation.tpcds.TPCDSOutputSchemaSuite,org.apache.kyuubi.operation.tpcds.TPCDSDDLSuite


org.apache.kyuubi.operation.tpcds.TPCDSOutputSchemaSuite,org.apache.kyuubi.operation.tpcds.TPCDSDDLSuite

can we rename them to xxxTPCDSSuite then -DwildcardSuites=*TPCDSSuite for short?

yaooqinn · 2021-05-25T15:52:55Z

.github/workflows/master.yml

@@ -91,3 +85,43 @@ jobs:
          path: |
            **/target/unit-tests.log
            **/kyuubi-spark-sql-engine.log*
+
+  tpcds:
+    name: TPC-DS Schema Tests


not only schema? simply TPC-DS Tests?

yaooqinn · 2021-05-25T15:53:44Z

.github/workflows/master.yml

+          key: ${{ runner.os }}-maven-io-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-io-
+      - name: Run TPC-DS Schema Tests


yaooqinn · 2021-05-25T15:59:42Z

.travis.yml

-        - EXCLUDE_TAGS="org.apache.kyuubi.tags.ExtendedSQLTest,org.apache.kyuubi.tags.DataLakeTest"
-    - name: Test Kyuubi w/ -Pspark-3.1 -Pspark-hadoop-3.2 -Pkyuubi-extension-spark_3.1
+        - JOB="build"
+    - name: Test Kyuubi w/ -Pspark-master w/ Spark 3.2 nightly build


remove w/ Spark 3.2 nightly build？

pan3793 · 2021-05-26T01:57:08Z

Thanks for review, got it merged for v1.3.0, and will send following PRs for 1. nightly build; 2. TPC-DS test against Delta

pan3793 commented May 25, 2021

View reviewed changes

.github/workflows/master.yml Outdated Show resolved Hide resolved

pan3793 self-assigned this May 25, 2021

pan3793 marked this pull request as ready for review May 25, 2021 05:25

pan3793 requested review from ulysses-you and yaooqinn May 25, 2021 05:25

ulysses-you reviewed May 25, 2021

View reviewed changes

ulysses-you approved these changes May 25, 2021

View reviewed changes

ulysses-you closed this May 25, 2021

ulysses-you reopened this May 25, 2021

pan3793 force-pushed the delta branch from 7a3d557 to 45e28eb Compare May 25, 2021 06:22

yaooqinn reviewed May 25, 2021

View reviewed changes

pan3793 force-pushed the delta branch from ea01d6f to e2dffef Compare May 25, 2021 12:34

pan3793 changed the title ~~[TEST] [DELTA] Test detla 1.0.0 with Spark 3.1~~ [TEST] [DELTA] Refine CI and add test detla 1.0.0 with Spark 3.1 May 25, 2021

pan3793 force-pushed the delta branch from e2dffef to 4d67257 Compare May 25, 2021 14:31

pan3793 added 10 commits May 25, 2021 23:37

[TEST] [DELTA] Test detla 1.0.0 with Spark 3.1

3204ef4

seperate DataLakeTest

19b1b16

scalatest.exclude.tags

41a667e

align gh action and travis

a0ed837

seperate tpcds in GA

73532b8

travis

9d194c3

reduce redundant ci jobs

4e2d51c

rename spark-3.2-snapshot to spark-master

4b8b68f

rename tpcds suites

8337279

mvn --no-transfer-progress

e68ae14

pan3793 force-pushed the delta branch from b553b55 to e68ae14 Compare May 25, 2021 15:38

yaooqinn reviewed May 25, 2021

View reviewed changes

address comments

e15a3ff

yaooqinn approved these changes May 25, 2021

View reviewed changes

yaooqinn added this to the v1.3.0 milestone May 25, 2021

pan3793 added the Test label May 26, 2021

pan3793 merged commit b567453 into apache:master May 26, 2021

pan3793 deleted the delta branch May 26, 2021 02:03

pan3793 added release highlight integration labels Sep 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEST] [DELTA] Refine CI and add test detla 1.0.0 with Spark 3.1 #643

[TEST] [DELTA] Refine CI and add test detla 1.0.0 with Spark 3.1 #643

pan3793 commented May 25, 2021 •

edited

Loading

pan3793 May 25, 2021

ulysses-you May 25, 2021

pan3793 May 25, 2021

codecov-commenter commented May 25, 2021 •

edited

Loading

pan3793 commented May 25, 2021

ulysses-you left a comment

ulysses-you May 25, 2021

pan3793 May 25, 2021

ulysses-you May 25, 2021

pan3793 May 25, 2021

ulysses-you May 25, 2021

ulysses-you May 25, 2021

ulysses-you commented May 25, 2021

yaooqinn May 25, 2021

yaooqinn May 25, 2021

yaooqinn May 25, 2021

pan3793 May 25, 2021

yaooqinn May 25, 2021

yaooqinn May 25, 2021

yaooqinn May 25, 2021

yaooqinn May 25, 2021

pan3793 commented May 26, 2021

[TEST] [DELTA] Refine CI and add test detla 1.0.0 with Spark 3.1 #643

[TEST] [DELTA] Refine CI and add test detla 1.0.0 with Spark 3.1 #643

Conversation

pan3793 commented May 25, 2021 • edited Loading

Why are the changes needed?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented May 25, 2021 • edited Loading

Codecov Report

pan3793 commented May 25, 2021

ulysses-you left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulysses-you commented May 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pan3793 commented May 26, 2021

pan3793 commented May 25, 2021 •

edited

Loading

codecov-commenter commented May 25, 2021 •

edited

Loading