Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented May 3, 2017

What changes were proposed in this pull request?

This PR proposes three things as below:

  • This test looks not testing <=> and identical with the test above, ===. So, it removes the test.

    -   test("<=>") {
    -     checkAnswer(
    -      testData2.filter($"a" === 1),
    -      testData2.collect().toSeq.filter(r => r.getInt(0) == 1))
    -
    -    checkAnswer(
    -      testData2.filter($"a" === $"b"),
    -      testData2.collect().toSeq.filter(r => r.getInt(0) == r.getInt(1)))
    -   }
  • Replace the test title from =!= to <=>. It looks the test actually testing <=>.

    +  private lazy val nullData = Seq(
    +    (Some(1), Some(1)), (Some(1), Some(2)), (Some(1), None), (None, None)).toDF("a", "b")
    +
      ...
    -  test("=!=") {
    +  test("<=>") {
    -    val nullData = spark.createDataFrame(sparkContext.parallelize(
    -      Row(1, 1) ::
    -      Row(1, 2) ::
    -      Row(1, null) ::
    -      Row(null, null) :: Nil),
    -      StructType(Seq(StructField("a", IntegerType), StructField("b", IntegerType))))
    -
           checkAnswer(
             nullData.filter($"b" <=> 1),
      ...
  • Add the tests for =!= which looks not existing.

    +  test("=!=") {
    +    checkAnswer(
    +      nullData.filter($"b" =!= 1),
    +      Row(1, 2) :: Nil)
    +
    +    checkAnswer(nullData.filter($"b" =!= null), Nil)
    +
    +    checkAnswer(
    +      nullData.filter($"a" =!= $"b"),
    +      Row(1, 2) :: Nil)
    +  }

How was this patch tested?

Manually running the tests.

@HyukjinKwon
Copy link
Member Author

cc @rxin, it looks the last related change was made by you. Could you check if it makes sense please?

@SparkQA
Copy link

SparkQA commented May 3, 2017

Test build #76413 has finished for PR 17842 at commit c164b88.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented May 3, 2017

Merging in master/branch-2.2.

asfgit pushed a commit that referenced this pull request May 3, 2017
…test and add a test for =!=

## What changes were proposed in this pull request?

This PR proposes three things as below:

- This test looks not testing `<=>` and identical with the test above, `===`. So, it removes the test.

  ```diff
  -   test("<=>") {
  -     checkAnswer(
  -      testData2.filter($"a" === 1),
  -      testData2.collect().toSeq.filter(r => r.getInt(0) == 1))
  -
  -    checkAnswer(
  -      testData2.filter($"a" === $"b"),
  -      testData2.collect().toSeq.filter(r => r.getInt(0) == r.getInt(1)))
  -   }
  ```

- Replace the test title from `=!=` to `<=>`. It looks the test actually testing `<=>`.

  ```diff
  +  private lazy val nullData = Seq(
  +    (Some(1), Some(1)), (Some(1), Some(2)), (Some(1), None), (None, None)).toDF("a", "b")
  +
    ...
  -  test("=!=") {
  +  test("<=>") {
  -    val nullData = spark.createDataFrame(sparkContext.parallelize(
  -      Row(1, 1) ::
  -      Row(1, 2) ::
  -      Row(1, null) ::
  -      Row(null, null) :: Nil),
  -      StructType(Seq(StructField("a", IntegerType), StructField("b", IntegerType))))
  -
         checkAnswer(
           nullData.filter($"b" <=> 1),
    ...
  ```

- Add the tests for `=!=` which looks not existing.

  ```diff
  +  test("=!=") {
  +    checkAnswer(
  +      nullData.filter($"b" =!= 1),
  +      Row(1, 2) :: Nil)
  +
  +    checkAnswer(nullData.filter($"b" =!= null), Nil)
  +
  +    checkAnswer(
  +      nullData.filter($"a" =!= $"b"),
  +      Row(1, 2) :: Nil)
  +  }
  ```

## How was this patch tested?

Manually running the tests.

Author: hyukjinkwon <[email protected]>

Closes #17842 from HyukjinKwon/minor-test-fix.

(cherry picked from commit 13eb37c)
Signed-off-by: Reynold Xin <[email protected]>
@asfgit asfgit closed this in 13eb37c May 3, 2017
@HyukjinKwon HyukjinKwon deleted the minor-test-fix branch January 2, 2018 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants