Skip to content

[SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite#45460

Closed
xi-db wants to merge 1 commit intoapache:masterfrom
xi-db:SPARK-47341-fix-misleading-tests
Closed

[SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite#45460
xi-db wants to merge 1 commit intoapache:masterfrom
xi-db:SPARK-47341-fix-misleading-tests

Conversation

@xi-db
Copy link
Contributor

@xi-db xi-db commented Mar 11, 2024

What changes were proposed in this pull request?

A few tests in SparkConnectClientSuite attempt to test the result collection of a reattachable execution through the use of a SQL command. The SQL command, on a real server, is not executed eagerly (since it is a select command) and thus, is not entirely accurate. The test itself is non-problematic since a dummy server with dummy responses is used but a small improvement here would be to construct a relation rather than a command.

Why are the changes needed?

Although these tests are not problematic, we should ensure that the behavior of the tests is consistent with the actual behavior of real servers to avoid misleading developers.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

The tests in org.apache.spark.sql.connect.client.SparkConnectClientSuite passed.

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

No.

@xi-db xi-db changed the title [WIP][SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite [SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite Mar 11, 2024
@xi-db xi-db changed the title [SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite [WIP][SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite Mar 11, 2024
Copy link
Contributor

@vicennial vicennial left a comment

Choose a reason for hiding this comment

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

LGTM

@xi-db xi-db changed the title [WIP][SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite [SPARK-47341][Connect] Replace commands with relations in a few tests in SparkConnectClientSuite Mar 11, 2024
@HyukjinKwon
Copy link
Member

Merged to master.

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.

3 participants

Comments