Skip to content

[SPARK-49579][SQL][TESTS][FOLLOWUP] Use condition instead of errorClass in SparkThrowableSuite and in DateTimeFormatterHelperSuite#48061

Closed
MaxGekk wants to merge 1 commit into
apache:masterfrom
MaxGekk:fix-missing-errorClass
Closed

[SPARK-49579][SQL][TESTS][FOLLOWUP] Use condition instead of errorClass in SparkThrowableSuite and in DateTimeFormatterHelperSuite#48061
MaxGekk wants to merge 1 commit into
apache:masterfrom
MaxGekk:fix-missing-errorClass

Conversation

@MaxGekk

@MaxGekk MaxGekk commented Sep 10, 2024

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

In the PR, I propose to use condition instead of errorClass in two test suites:

  • SparkThrowableSuite
  • DateTimeFormatterHelperSuite

Why are the changes needed?

Because the changes from the PR #48027 conflict to #48058 and #48026, and tests in #48027 were passed earlier than the last PRs were merged to master.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

By compiling and running the following tests locally:

$ build/sbt "test:testOnly *org.apache.spark.sql.catalyst.util.DateTimeFormatterHelperSuite"
$ build/sbt "test:testOnly *SparkThrowableSuite"

Was this patch authored or co-authored using generative AI tooling?

No.

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, LGTM.

@dongjoon-hyun

Copy link
Copy Markdown
Member

Feel free to merge this if you verified locally, @MaxGekk .

@MaxGekk

MaxGekk commented Sep 10, 2024

Copy link
Copy Markdown
Member Author

@dongjoon-hyun I have tested the changes locally, but let's wait for a few minutes till compilation in GAs is done.

@MaxGekk

MaxGekk commented Sep 10, 2024

Copy link
Copy Markdown
Member Author

Merging to master. Thank you, @dongjoon-hyun for the quick review.

@MaxGekk MaxGekk closed this in e0a2c74 Sep 10, 2024
@dongjoon-hyun

Copy link
Copy Markdown
Member

Thank you for quick follow-up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants