Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jan 7, 2025

What changes were proposed in this pull request?

This PR aims to fix comments on runIfTestsEnabled methods.

Why are the changes needed?

Apache Spark repository has two runIfTestsEnabled methods whose comments are wrong or ambiguous.

$ git grep -C1 'def runIfTestsEnabled'
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/DockerIntegrationFunSuite.scala-  /** Run the give body of code only if Kinesis tests are enabled */
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/DockerIntegrationFunSuite.scala:  def runIfTestsEnabled(message: String)(body: => Unit): Unit = {
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/DockerIntegrationFunSuite.scala-    if (shouldRunTests) {
--
connector/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisFunSuite.scala-  /** Run the give body of code only if Kinesis tests are enabled */
connector/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisFunSuite.scala:  def runIfTestsEnabled(message: String)(body: => Unit): Unit = {
connector/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisFunSuite.scala-    if (shouldRunTests) {

Does this PR introduce any user-facing change?

No, this is a comment revision.

How was this patch tested?

Manual review.

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

No.

}
}

/** Run the give body of code only if Kinesis tests are enabled */
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not Kinesis test.

}

/** Run the give body of code only if Kinesis tests are enabled */
/** Run the given body of code only if ENABLE_KINESIS_TESTS is 1. */
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix typo and make it explicit by mentioning the env variable name.

@dongjoon-hyun dongjoon-hyun changed the title [MINOR][TESTS] Fix comments on runIfTestsEnabled methods [MINOR][TESTS] Fix comments on runIfTestsEnabled methods Jan 7, 2025
@dongjoon-hyun
Copy link
Member Author

Could you review this a minor comment fix, @yaooqinn ?

@LuciferYang
Copy link
Contributor

Merged into master. Thanks @dongjoon-hyun

@yaooqinn
Copy link
Member

yaooqinn commented Jan 7, 2025

Late LGTM

@dongjoon-hyun
Copy link
Member Author

Thank you, @LuciferYang and @yaooqinn !

@dongjoon-hyun dongjoon-hyun deleted the minor2 branch January 7, 2025 14:27
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