[SPARK-32292][SPARK-32252][INFRA] Run the relevant tests only in GitHub Actions #29086

HyukjinKwon · 2020-07-13T11:01:06Z

What changes were proposed in this pull request?

This PR mainly proposes to run only relevant tests just like Jenkins PR builder does. Currently, GitHub Actions always run full tests which wastes the resources.

In addition, this PR also fixes 3 more issues very closely related together while I am here.

The main idea here is: It reuses the existing logic embedded in dev/run-tests.py which Jenkins PR builder use in order to run only the related test cases.
While I am here, I fixed SPARK-32292 too to run the doc tests. It was because other references were not available when it is cloned via checkout@v2. With fetch-depth: 0, the history is available.
In addition, it fixes the dev/run-tests.py to match with python/run-tests.py in terms of its options. Environment variables such as TEST_ONLY_XXX were moved as proper options. For example,
```
dev/run-tests.py --modules sql,core
```
which is consistent with python/run-tests.py, for example,
```
python/run-tests.py --modules pyspark-core,pyspark-ml
```

Lastly, also fixed the formatting issue in module specification in the matrix:

-            network_common, network_shuffle, repl, launcher
+            network-common, network-shuffle, repl, launcher,

which incorrectly runs build/test the modules.

Why are the changes needed?

By running only related tests, we can hugely save the resources and avoid unrelated flaky tests, etc.
Also, now it runs the doctest of dev/run-tests.py properly, the usages are similar between dev/run-tests.py and python/run-tests.py, and run network-common, network-shuffle, launcher and examples modules too.

Does this PR introduce any user-facing change?

No, dev-only.

How was this patch tested?

Manually tested in my own forked Spark:

HyukjinKwon#7
HyukjinKwon#8
HyukjinKwon#9
HyukjinKwon#10
HyukjinKwon#11
HyukjinKwon#12

HyukjinKwon · 2020-07-13T11:07:40Z

dev/run-tests.py

+        # If we're running the tests in Github Actions, attempt to detect and test
+        # only the affected modules.
+        if test_env == "github_actions":
+            if os.environ["GITHUB_BASE_REF"] != "":


GITHUB_BASE_REF is only set in the PR builder; otherwise, it's an empty string.

~~Do you mean Jenkins PR Builder?~~

HyukjinKwon · 2020-07-13T11:08:52Z

dev/run-tests.py

+            if os.environ["GITHUB_BASE_REF"] != "":
+                # Pull requests
+                changed_files = identify_changed_files_from_git_commits(
+                    os.environ["GITHUB_SHA"], target_branch=os.environ["GITHUB_BASE_REF"])


GITHUB_SHA is the commit in PR. GITHUB_BASE_REF is the target repo's branch.

For many commits in a PR, each time changed_files is different? If a previous commit changed two modules but later one changes only one, will this only run test for the later module?

From my testing, it combines all commits and changed_files is based on all commits always.

Seems like GITHUB_SHA became a merge commit internally in Github Actions, and GITHUB_BASE_REF becomes the branch which PR opens against.

For example, if a PR has commits 1, 2 and 3, GitHub Actions looks creates a merge commit 4 for commits 1, 2 and 3. And, it calculates the diff from the branch, for example, master in the original repo, Apache Spark.

git diff 4 master

This is an example of the merge commit: HyukjinKwon@8f36ec4 at HyukjinKwon#7

Okay. Thanks for clarifying. Looks good.

SparkQA · 2020-07-13T14:09:32Z

Test build #125769 has finished for PR 29086 at commit 9d8555a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-07-13T15:26:12Z

.github/workflows/master.yml

@@ -29,7 +29,7 @@ jobs:
        modules:
          - |-
            core, unsafe, kvstore, avro,
-            network_common, network_shuffle, repl, launcher
+            network-common, network-shuffle, repl, launcher,


Oops. I missed this.

dongjoon-hyun · 2020-07-13T15:30:53Z

dev/run-tests.py

+
+        # If we're running the tests in Github Actions, attempt to detect and test
+        # only the affected modules.
+        if test_env == "github_actions":


Thanks. This looks safe.

dongjoon-hyun

+1, LGTM. Thanks, @HyukjinKwon .
Merged to master.

HyukjinKwon · 2020-07-13T15:35:28Z

Thank you @dongjoon-hyun!

…ub Actions This PR mainly proposes to run only relevant tests just like Jenkins PR builder does. Currently, GitHub Actions always run full tests which wastes the resources. In addition, this PR also fixes 3 more issues very closely related together while I am here. 1. The main idea here is: It reuses the existing logic embedded in `dev/run-tests.py` which Jenkins PR builder use in order to run only the related test cases. 2. While I am here, I fixed SPARK-32292 too to run the doc tests. It was because other references were not available when it is cloned via `checkoutv2`. With `fetch-depth: 0`, the history is available. 3. In addition, it fixes the `dev/run-tests.py` to match with `python/run-tests.py` in terms of its options. Environment variables such as `TEST_ONLY_XXX` were moved as proper options. For example, ```bash dev/run-tests.py --modules sql,core ``` which is consistent with `python/run-tests.py`, for example, ```bash python/run-tests.py --modules pyspark-core,pyspark-ml ``` 4. Lastly, also fixed the formatting issue in module specification in the matrix: ```diff - network_common, network_shuffle, repl, launcher + network-common, network-shuffle, repl, launcher, ``` which incorrectly runs build/test the modules. By running only related tests, we can hugely save the resources and avoid unrelated flaky tests, etc. Also, now it runs the doctest of `dev/run-tests.py` properly, the usages are similar between `dev/run-tests.py` and `python/run-tests.py`, and run `network-common`, `network-shuffle`, `launcher` and `examples` modules too. No, dev-only. Manually tested in my own forked Spark: #7 #8 #9 #10 #11 #12 Closes apache#29086 from HyukjinKwon/SPARK-32292. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

Run the relavent tests only in GitHub Actions

9d8555a

probot-autolabeler bot added BUILD INFRA labels Jul 13, 2020

HyukjinKwon force-pushed the SPARK-32292 branch from d70e6a6 to 9d8555a Compare July 13, 2020 11:01

HyukjinKwon requested review from dbtsai, dongjoon-hyun and gengliangwang July 13, 2020 11:01

HyukjinKwon commented Jul 13, 2020

View reviewed changes

dongjoon-hyun reviewed Jul 13, 2020

View reviewed changes

dongjoon-hyun approved these changes Jul 13, 2020

View reviewed changes

dongjoon-hyun closed this in 27ef362 Jul 13, 2020

HyukjinKwon deleted the SPARK-32292 branch July 27, 2020 07:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-32292][SPARK-32252][INFRA] Run the relevant tests only in GitHub Actions #29086

[SPARK-32292][SPARK-32252][INFRA] Run the relevant tests only in GitHub Actions #29086

HyukjinKwon commented Jul 13, 2020 •

edited

Loading

HyukjinKwon Jul 13, 2020

dongjoon-hyun Jul 13, 2020 •

edited

Loading

HyukjinKwon Jul 13, 2020

viirya Jul 14, 2020

HyukjinKwon Jul 14, 2020

HyukjinKwon Jul 14, 2020

viirya Jul 14, 2020

SparkQA commented Jul 13, 2020

dongjoon-hyun Jul 13, 2020 •

edited

Loading

dongjoon-hyun Jul 13, 2020

dongjoon-hyun left a comment

HyukjinKwon commented Jul 13, 2020

[SPARK-32292][SPARK-32252][INFRA] Run the relevant tests only in GitHub Actions #29086

[SPARK-32292][SPARK-32252][INFRA] Run the relevant tests only in GitHub Actions #29086

Conversation

HyukjinKwon commented Jul 13, 2020 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

HyukjinKwon Jul 13, 2020

Choose a reason for hiding this comment

dongjoon-hyun Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

HyukjinKwon Jul 13, 2020

Choose a reason for hiding this comment

viirya Jul 14, 2020

Choose a reason for hiding this comment

HyukjinKwon Jul 14, 2020

Choose a reason for hiding this comment

HyukjinKwon Jul 14, 2020

Choose a reason for hiding this comment

viirya Jul 14, 2020

Choose a reason for hiding this comment

SparkQA commented Jul 13, 2020

dongjoon-hyun Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

dongjoon-hyun Jul 13, 2020

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

HyukjinKwon commented Jul 13, 2020

HyukjinKwon commented Jul 13, 2020 •

edited

Loading

dongjoon-hyun Jul 13, 2020 •

edited

Loading

dongjoon-hyun Jul 13, 2020 •

edited

Loading