Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Nov 14, 2022

What changes were proposed in this pull request?

This pr aims to fix error class order of ESC_IN_THE_MIDDLE and ESC_AT_THE_END to make GA task passed.

Why are the changes needed?

Fix GA test task failed.

Does this PR introduce any user-facing change?

No

How was this patch tested?

  • Pass GA
  • Manual test:
build/sbt "core/testOnly *SparkThrowableSuite"

Before

[info] - Error classes are correctly formatted *** FAILED *** (91 milliseconds)
[info]   "...ass" : {
[info]         "ESC_[AT_THE_END" : {
[info]           "message" : [
[info]             "the escape character is not allowed to end with."
[info]           ]
[info]         },
[info]         "ESC_IN_THE_MIDDLE" : {
[info]           "message" : [
[info]             "the escape character is not allowed to precede <char>]."
[info]           ]
[info]         }..." did not equal "...ass" : {
[info]         "ESC_[IN_THE_MIDDLE" : {
[info]           "message" : [
[info]             "the escape character is not allowed to precede <char>."
[info]           ]
[info]         },
[info]         "ESC_AT_THE_END" : {
[info]           "message" : [
[info]             "the escape character is not allowed to end with]."
[info]           ]
[info]         }..." (SparkThrowableSuite.scala:98)

After

[info] SparkThrowableSuite:
[info] - No duplicate error classes (39 milliseconds)
[info] - Error classes are correctly formatted (61 milliseconds)
[info] - SQLSTATE invariants (13 milliseconds)
[info] - Message invariants (15 milliseconds)
[info] - Message format invariants (33 milliseconds)
[info] - Round trip (33 milliseconds)
[info] - Check if error class is missing (32 milliseconds)
[info] - Check if message parameters match message format (8 milliseconds)
[info] - Error message is formatted (1 millisecond)
[info] - Try catching legacy SparkError (1 millisecond)
[info] - Try catching SparkError with error class (1 millisecond)
[info] - Try catching internal SparkError (1 millisecond)
[info] - Get message in the specified format (15 milliseconds)
[info] - overwrite error classes (190 milliseconds)
[info] - prohibit dots in error class names (57 milliseconds)
[info] Run completed in 2 seconds, 317 milliseconds.
[info] Total number of tests run: 15
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 15, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 16 s, completed 2022-11-14 19:34:11

@github-actions github-actions bot added the CORE label Nov 14, 2022
@LuciferYang
Copy link
Contributor Author

LuciferYang commented Nov 14, 2022

cc @MaxGekk @dongjoon-hyun @HyukjinKwon @srowen @panbingkun

for fix GA Task:

https://github.com/LuciferYang/spark/actions/runs/3459895559/jobs/5775820010

2022-11-14T10:03:58.9326192Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- No duplicate error classes (68 milliseconds)�[0m�[0m
2022-11-14T10:03:59.1034282Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m- Error classes are correctly formatted *** FAILED *** (139 milliseconds)�[0m�[0m
2022-11-14T10:03:59.1035227Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m  "...ass" : {�[0m�[0m
2022-11-14T10:03:59.1036507Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        "ESC_[AT_THE_END" : {�[0m�[0m
2022-11-14T10:03:59.1036881Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          "message" : [�[0m�[0m
2022-11-14T10:03:59.1037473Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m            "the escape character is not allowed to end with."�[0m�[0m
2022-11-14T10:03:59.1037936Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          ]�[0m�[0m
2022-11-14T10:03:59.1040627Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        },�[0m�[0m
2022-11-14T10:03:59.1041002Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        "ESC_IN_THE_MIDDLE" : {�[0m�[0m
2022-11-14T10:03:59.1041379Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          "message" : [�[0m�[0m
2022-11-14T10:03:59.1041831Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m            "the escape character is not allowed to precede <char>]."�[0m�[0m
2022-11-14T10:03:59.1042247Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          ]�[0m�[0m
2022-11-14T10:03:59.1042644Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        }..." did not equal "...ass" : {�[0m�[0m
2022-11-14T10:03:59.1043133Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        "ESC_[IN_THE_MIDDLE" : {�[0m�[0m
2022-11-14T10:03:59.1048200Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          "message" : [�[0m�[0m
2022-11-14T10:03:59.1049028Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m            "the escape character is not allowed to precede <char>."�[0m�[0m
2022-11-14T10:03:59.1049610Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          ]�[0m�[0m
2022-11-14T10:03:59.1049940Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        },�[0m�[0m
2022-11-14T10:03:59.1050301Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        "ESC_AT_THE_END" : {�[0m�[0m
2022-11-14T10:03:59.1050672Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          "message" : [�[0m�[0m
2022-11-14T10:03:59.1051104Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m            "the escape character is not allowed to end with]."�[0m�[0m
2022-11-14T10:03:59.1051471Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m          ]�[0m�[0m
2022-11-14T10:03:59.1054378Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[31m        }..." (SparkThrowableSuite.scala:98)�[0m�[0m

@LuciferYang LuciferYang changed the title [SPARK-41109][CORE][FOLLOWUP] Fix error class order [SPARK-41109][CORE][FOLLOWUP] Re-order error class to fix SparkThrowableSuite Nov 14, 2022
@LuciferYang LuciferYang changed the title [SPARK-41109][CORE][FOLLOWUP] Re-order error class to fix SparkThrowableSuite [SPARK-41109][CORE][FOLLOWUP] Re-order error class to fix SparkThrowableSuite Nov 14, 2022
@itholic
Copy link
Contributor

itholic commented Nov 14, 2022

Thanks for the fix this.

Do you know why this suddenly start failing?

@LuciferYang
Copy link
Contributor Author

Still under investigation. It should have been discovered very early

Copy link
Contributor

@itholic itholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM when test passed

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Nov 14, 2022

Thanks for the fix this.

Do you know why this suddenly start failing?

@itholic

It seems that incremental testing is triggered after code merging into the master.

After #38615 merging, there is a code style issue, which is fixed by #38645.

However, the merging of #38645 and #38626 is not determined to have core module changes.

Therefore, the testing of core module is not triggered after code merging into the master and noSparkThrowableSuite test failure is triggered.

The evidence is that I found the following log in the latest Run tests log,

2022-11-14T08:09:17.9465991Z [info] Found the following changed modules: repl, avro, examples

and only tested repl, avro this time.

https://pipelines.actions.githubusercontent.com/serviceHosts/03398d36-4378-4d47-a936-fba0a5e8ccb9/_apis/pipelines/1/runs/195451/signedlogcontent/16?urlExpires=2022-11-14T13%3A04%3A29.9673237Z&urlSigningMethod=HMACV1&urlSignature=LG43JDLy%2B3THPnt81QbCb3qSfW9kus%2FIT1QXEPzwYm4%3D

@LuciferYang
Copy link
Contributor Author

friendly ping @cloud-fan

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... to make GA task failed.

@LuciferYang In the PR, could you change this to to make GA task passed.

@MaxGekk
Copy link
Member

MaxGekk commented Nov 14, 2022

+1, LGTM. Merging to master. I have checked the test suite locally:

[info] SparkThrowableSuite:
...
[info] - prohibit dots in error class names (87 milliseconds)
[info] Run completed in 2 seconds, 823 milliseconds.
[info] Total number of tests run: 15
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 15, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed

Thank you, @LuciferYang and @cloud-fan @itholic for review.

@MaxGekk MaxGekk closed this in 5ff060e Nov 14, 2022
@LuciferYang
Copy link
Contributor Author

Thanks @MaxGekk @cloud-fan @itholic

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. I was actually looking into these errors.

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
…ableSuite`

### What changes were proposed in this pull request?
This pr aims to fix error class order of `ESC_IN_THE_MIDDLE` and `ESC_AT_THE_END` to make GA task passed.

### Why are the changes needed?
Fix GA test task failed.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
- Pass GA
- Manual test:

```
build/sbt "core/testOnly *SparkThrowableSuite"
```

**Before**

```
[info] - Error classes are correctly formatted *** FAILED *** (91 milliseconds)
[info]   "...ass" : {
[info]         "ESC_[AT_THE_END" : {
[info]           "message" : [
[info]             "the escape character is not allowed to end with."
[info]           ]
[info]         },
[info]         "ESC_IN_THE_MIDDLE" : {
[info]           "message" : [
[info]             "the escape character is not allowed to precede <char>]."
[info]           ]
[info]         }..." did not equal "...ass" : {
[info]         "ESC_[IN_THE_MIDDLE" : {
[info]           "message" : [
[info]             "the escape character is not allowed to precede <char>."
[info]           ]
[info]         },
[info]         "ESC_AT_THE_END" : {
[info]           "message" : [
[info]             "the escape character is not allowed to end with]."
[info]           ]
[info]         }..." (SparkThrowableSuite.scala:98)
```
**After**

```
[info] SparkThrowableSuite:
[info] - No duplicate error classes (39 milliseconds)
[info] - Error classes are correctly formatted (61 milliseconds)
[info] - SQLSTATE invariants (13 milliseconds)
[info] - Message invariants (15 milliseconds)
[info] - Message format invariants (33 milliseconds)
[info] - Round trip (33 milliseconds)
[info] - Check if error class is missing (32 milliseconds)
[info] - Check if message parameters match message format (8 milliseconds)
[info] - Error message is formatted (1 millisecond)
[info] - Try catching legacy SparkError (1 millisecond)
[info] - Try catching SparkError with error class (1 millisecond)
[info] - Try catching internal SparkError (1 millisecond)
[info] - Get message in the specified format (15 milliseconds)
[info] - overwrite error classes (190 milliseconds)
[info] - prohibit dots in error class names (57 milliseconds)
[info] Run completed in 2 seconds, 317 milliseconds.
[info] Total number of tests run: 15
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 15, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 16 s, completed 2022-11-14 19:34:11
```

Closes apache#38658 from LuciferYang/SPARK-41109-FOLLOWUP.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants