Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

What changes were proposed in this pull request?

This is a follow-up of #22313 and aim to ignore the micro benchmark test which takes over 2 minutes in Jenkins.

How was this patch tested?

The test case should be ignored in Jenkins.

[info] FilterPushdownBenchmark:
...
[info] - Pushdown benchmark with many filters !!! IGNORED !!!

@dongjoon-hyun
Copy link
Member Author

Sorry, @cloud-fan . I forgot to turn off the benchmark test in the previous PR. We need to disable it like the other micro benchmark test.

@cloud-fan
Copy link
Contributor

cloud-fan commented Sep 5, 2018

I'm surprised this benchmark is written as a test suite.

I'm ok with this PR, but later we should refactor this benchmark to use main method, like HashBenchmark.

@kiszk
Copy link
Member

kiszk commented Sep 5, 2018

LGTM

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Sep 5, 2018

Yep. I see. I'll refactor it later, @cloud-fan . Also, thanks, @kiszk and @HyukjinKwon

@SparkQA
Copy link

SparkQA commented Sep 5, 2018

Test build #95693 has finished for PR 22336 at commit 69f207f.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Sep 5, 2018

Test build #95699 has finished for PR 22336 at commit 69f207f.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member

kiszk commented Sep 5, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Sep 5, 2018

Test build #95710 has finished for PR 22336 at commit 69f207f.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Sep 5, 2018

Test build #95722 has finished for PR 22336 at commit 69f207f.

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

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Sep 5, 2018

Finally, it's passed. Shall I merge this, @cloud-fan and @HyukjinKwon ? :)

@gatorsmile
Copy link
Member

gatorsmile commented Sep 5, 2018

Sorry,,, I just merged it to master

@dongjoon-hyun
Copy link
Member Author

Wow, Great. Thanks, @gatorsmile !

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.

6 participants