Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Build] Test selective testing #2441

Closed
wants to merge 8 commits into from

Conversation

nchammas
Copy link
Contributor

This is a dummy PR to test the work done in #2420 and #2437.

Do not merge it.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have started for PR 2441 at commit 17505de.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have started for PR 2441 at commit 74266ab.

  • This patch merges cleanly.

This reverts commit 17505de.
@nchammas
Copy link
Contributor Author

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have started for PR 2441 at commit 97c43a9.

  • This patch merges cleanly.

@nchammas
Copy link
Contributor Author

@marmbrus @pwendell @rxin

Here are the Jenkins outputs for when:

Please confirm they look good.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have finished for PR 2441 at commit 17505de.

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

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have finished for PR 2441 at commit 74266ab.

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

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have finished for PR 2441 at commit 97c43a9.

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

@marmbrus
Copy link
Contributor

Overall I think this is better, thanks! One minor issue is that hive-thriftserver is missing from the SQL tests (Sorry, I think I left this out when you asked).

Only SQL files have been modified
SQL and non-SQL files have been modified
Only non-SQL files have been modified

    1. looks good to me
    1. runs all tests, hive is included
    1. runs all tests, hive is not included

Is there a reason for the difference in the 2 & 3? I think its okay to only run hive tests when catalyst, sql, or infrastructure (scripts, build files, etc) changes. Though I'll let others chime in here too.

/cc @liancheng @JoshRosen

@nchammas
Copy link
Contributor Author

One minor issue is that hive-thriftserver is missing from the SQL tests

No problem. Just give me an updated list of arguments to pass to sbt when you just want to test SQL components, and I'll submit a new PR to fix that.

Is there a reason for the difference in the 2 & 3?

I'm not sure; I believe this was the behavior from before my patch.

@marmbrus
Copy link
Contributor

One minor issue is that hive-thriftserver is missing from the SQL tests

No problem. Just give me an updated list of arguments to pass to sbt when you just want to test SQL components, and I'll submit a new PR to fix that.

Just add hive-thriftserver/test to the sbt command for SQL.

Is there a reason for the difference in the 2 & 3?

I'm not sure; I believe this was the behavior from before my patch.

Sorry, I think I'm confused and this is actually correct as is.

nchammas added a commit to nchammas/spark that referenced this pull request Sep 18, 2014
Addresses the problem pointed out in [this comment](apache#2441 (comment)).
@nchammas
Copy link
Contributor Author

Just add hive-thriftserver/test to the sbt command for SQL.

Done in #2442.

@nchammas nchammas closed this Sep 18, 2014
asfgit pushed a commit that referenced this pull request Sep 18, 2014
Addresses the problem pointed out in [this comment](#2441 (comment)).

Author: Nicholas Chammas <[email protected]>

Closes #2442 from nchammas/patch-1 and squashes the following commits:

7e68b60 [Nicholas Chammas] [SPARK-3534] Add hive-thriftserver to SQL tests
@nchammas nchammas deleted the test-selective-testing branch September 18, 2014 15:49
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.

3 participants