You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/16 04:27:07 UTC
[GitHub] [spark] EnricoMi opened a new pull request, #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
EnricoMi opened a new pull request, #36413:
URL: https://github.com/apache/spark/pull/36413
### What changes were proposed in this pull request?
The CI should not fail when there are no test result files to be downloaded.
### Why are the changes needed?
The CI workflow "Report test results" fails when there are no artifacts to be downloaded from the triggering workflow. In some situations, the triggering workflow is not skipped, but all test jobs are skipped in case no code changes are detected.
Examples:
- https://github.com/G-Research/spark/runs/6208072225?check_suite_focus=true#step:4:14
- https://github.com/G-Research/spark/runs/6094719451?check_suite_focus=true#step:4:14
In that situation, no test files are uploaded, which makes the triggered workflow fail.
Downloading no test files can have two reasons:
1. The code has been build but no tests have been executed, or tests have been executed but no test files have been generated.
3. No code has been built and tested deliberately.
You want to be notified in the first situation to fix the CI. Therefore, CI should fail when code is built and tests are run but no test result files are been found.
The `dawidd6/action-download-artifact` action cannot be configured to not fail when there are no files. But Github Actions can be configured (`continue-on-error`) to ignore that and still have the job succeed. However, if that action fails for a different reason, this will not be raised as an error any more.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Not tested
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
srowen commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r896907645
##########
.github/workflows/build_and_test.yml:
##########
@@ -280,6 +280,7 @@ jobs:
with:
name: test-results-${{ matrix.modules }}-${{ matrix.comment }}-${{ matrix.java }}-${{ matrix.hadoop }}-${{ matrix.hive }}
path: "**/target/test-reports/*.xml"
+ if-no-files-found: error
Review Comment:
I think this is OK? @HyukjinKwon
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen closed pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
srowen closed pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
URL: https://github.com/apache/spark/pull/36413
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r862659269
##########
.github/workflows/build_and_test.yml:
##########
@@ -280,6 +280,7 @@ jobs:
with:
name: test-results-${{ matrix.modules }}-${{ matrix.comment }}-${{ matrix.java }}-${{ matrix.hadoop }}-${{ matrix.hive }}
path: "**/target/test-reports/*.xml"
+ if-no-files-found: error
Review Comment:
You refer to case `1.` in the description above?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r897427925
##########
.github/workflows/build_and_test.yml:
##########
@@ -280,6 +280,7 @@ jobs:
with:
name: test-results-${{ matrix.modules }}-${{ matrix.comment }}-${{ matrix.java }}-${{ matrix.hadoop }}-${{ matrix.hive }}
path: "**/target/test-reports/*.xml"
+ if-no-files-found: error
Review Comment:
We can try and go ahead, I'm not against this. My only concern is that we'll now consider the build as failed in the case when the build is successful but it failed to generate XML test reports for an unknown reason - this happens often as far as I remember. Though, I agree that this PR can make such unknown cases onto the surface.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1157361682
Yes, it looks like the scope of the `if` condition of the `build` jobs is broader than the logic in `dev/run-tests.py`. The individual `build` jobs test individual sets of modules (e.g. module `sql`). They should be skipped if none of these modules have changed.
The `Check changes` job creates the `build` output, which covers modules of all `build` jobs and is used to skip all `build` jobs. The individual `build` jobs test far smaller sets of modules, so there may always be modules skipped by `dev/run-tests.py` though `build` is `true`.
This looks like a bigger 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r862695108
##########
.github/workflows/build_and_test.yml:
##########
@@ -280,6 +280,7 @@ jobs:
with:
name: test-results-${{ matrix.modules }}-${{ matrix.comment }}-${{ matrix.java }}-${{ matrix.hadoop }}-${{ matrix.hive }}
path: "**/target/test-reports/*.xml"
+ if-no-files-found: error
Review Comment:
Oh no. I meant that there's a case when the tests actually run but XML files are not generated for some reasons I don't know IIRC.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen closed pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
srowen closed pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
URL: https://github.com/apache/spark/pull/36413
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1113895897
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1157218266
Ah shoot OK thank you. You were right
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r862553504
##########
.github/workflows/build_and_test.yml:
##########
@@ -280,6 +280,7 @@ jobs:
with:
name: test-results-${{ matrix.modules }}-${{ matrix.comment }}-${{ matrix.java }}-${{ matrix.hadoop }}-${{ matrix.hive }}
path: "**/target/test-reports/*.xml"
+ if-no-files-found: error
Review Comment:
Hm, there's a case when *.xml are not generated time to time although when the tests passed. My only concern is that to make that case also failed after this 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r862705748
##########
.github/workflows/build_and_test.yml:
##########
@@ -280,6 +280,7 @@ jobs:
with:
name: test-results-${{ matrix.modules }}-${{ matrix.comment }}-${{ matrix.java }}-${{ matrix.hadoop }}-${{ matrix.hive }}
path: "**/target/test-reports/*.xml"
+ if-no-files-found: error
Review Comment:
I think that is what I meant with `1.`. I have reworded that to be more clear:
> 1. The code has been build […] tests have been executed but no test files have been generated.
This is covered by `if-no-files-found: error` for `actions/upload-artifact`. The upload action is triggered when tests are run, so it expects files to upload.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1157219031
that's fine. i think it's good that we know there's a problem there 👍
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1157205469
Seems like it fails in some cases such as https://github.com/apache/spark/commit/3b709ebf8aea0f0b21f5ee721900f88e3f5e8b4e
<img width="1019" alt="Screen Shot 2022-06-16 at 12 53 11 PM" src="https://user-images.githubusercontent.com/6477701/173987683-2ec9299c-0546-4e63-a196-e7868ef15c97.png">
The problem seems that https://github.com/apache/spark/blob/master/.github/workflows/build_and_test.yml#L134-L142 and actually testing skip logic are not matched.
For example, here https://github.com/apache/spark/runs/6911661666?check_suite_focus=true#step:4:14 it says to build both PySpark and other builds too but later other builds were skipped at https://github.com/apache/spark/runs/6911690622?check_suite_focus=true#step:9:21
We should probably remove pyspark-*, sparkr*, etc at https://github.com/apache/spark/blob/master/.github/workflows/build_and_test.yml#L135 but I think it needs a closer look.
Reverting this for now to avoid making other PRs failed.
cc @EnricoMi mind taking a look please?
cc @dongjoon-hyun too FYI
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1157273975
Thank you for the investigation, @HyukjinKwon .
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1156432328
Merged to master
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36413:
URL: https://github.com/apache/spark/pull/36413#discussion_r862553580
##########
.github/workflows/test_report.yml:
##########
@@ -31,6 +31,7 @@ jobs:
steps:
- name: Download test results to report
uses: dawidd6/action-download-artifact@09385b76de790122f4da9c82b17bccf858b9557c # pin@v2
+ continue-on-error: true
Review Comment:
This change might be fine.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] EnricoMi commented on pull request #36413: [SPARK-39074][CI] Fail on upload, not download of missing test files
Posted by GitBox <gi...@apache.org>.
EnricoMi commented on PR #36413:
URL: https://github.com/apache/spark/pull/36413#issuecomment-1113464100
@HyukjinKwon @Yikun there are still errors in forks when syncing main branch.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org