Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Aug 31, 2016

What changes were proposed in this pull request?

Currently, HiveContext in SparkR is not being tested and always skipped.
This is because the initiation of TestHiveContext is being failed due to trying to load non-existing data paths (test tables).

This is introduced from #14005

This enables the tests with SparkR.

How was this patch tested?

Manually,

Before (on Mac OS)

...
Skipped ------------------------------------------------------------------------
1. create DataFrame from RDD (@test_sparkSQL.R#200) - Hive is not build with SparkSQL, skipped
2. test HiveContext (@test_sparkSQL.R#1041) - Hive is not build with SparkSQL, skipped
3. read/write ORC files (@test_sparkSQL.R#1748) - Hive is not build with SparkSQL, skipped
4. enableHiveSupport on SparkSession (@test_sparkSQL.R#2480) - Hive is not build with SparkSQL, skipped
5. sparkJars tag in SparkContext (@test_Windows.R#21) - This test is only for Windows, skipped
...

After (on Mac OS)

...
Skipped ------------------------------------------------------------------------
1. sparkJars tag in SparkContext (@test_Windows.R#21) - This test is only for Windows, skipped
...

Please refer the tests below (on Windows)

@HyukjinKwon
Copy link
Member Author

cc @rxin, @felixcheung and @shivaram

@shivaram
Copy link
Contributor

Thanks @HyukjinKwon - This is a great catch. LGTM pending tests.

@felixcheung
Copy link
Member

LGTM thanks!

@SparkQA
Copy link

SparkQA commented Aug 31, 2016

Test build #64700 has finished for PR 14889 at commit 2cdcf4f.

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

@felixcheung
Copy link
Member

btw, do you know why the "skipped" message is not in the Jenkins log?

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Aug 31, 2016

@felixcheung I remember in case of PySpark/Scala, only fixed and related modules are tested by tracking the changed/related modules via changed files in the PR with Jenkins (within run-tests.py) but not too sure of SparkR one.

Ah, you meant just message within Skipped -------------------------------------------------------------------- block. I see. I will check this out.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Aug 31, 2016

It seems SparkR always runs the tests all? I will check this out.

@felixcheung
Copy link
Member

thanks for fixing this. I thought skipped tests are easy to miss so it would be great (maybe as a separate PR) if there is a way we could detect what's being skipped from within Jenkins.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Aug 31, 2016

@felixcheung It seems due to the differences of testthat version..? It seems this printing is added from testthat 1.0.0(https://github.com/hadley/testthat/blob/master/NEWS.md#testthat-100) (with this r-lib/testthat@57dc3ce)?

Do you mind if I ask what version is used in Jenkins (if you know)? It seems I can't find.

@felixcheung
Copy link
Member

felixcheung commented Aug 31, 2016 via email

@HyukjinKwon
Copy link
Member Author

It seems okay with minimal version. It seems 3.1+ (if my understanding is correct, https://github.com/hadley/testthat/blob/v1.0.0/DESCRIPTION#L13). I will try to look into this deeper.

@shivaram
Copy link
Contributor

I just ran this on one of the Jenkins machines

> packageVersion("testthat")
[1] ‘0.11.0’

@felixcheung
Copy link
Member

could we update testthat on Jenkins box? I think that would automagically bubble up the skipped test.
maybe then we could parse the log to check if we are not accidentally skipping more test over time?

@felixcheung
Copy link
Member

let's merge this fix and follow up if there is a better way to track skipped tests separately?

@shivaram
Copy link
Contributor

Yeah lets open a separate JIRA to update testthat on the Jenkins boxes. LGTM. Merging this to master and branch-2.0

asfgit pushed a commit that referenced this pull request Aug 31, 2016
…skipped always

## What changes were proposed in this pull request?

Currently, `HiveContext` in SparkR is not being tested and always skipped.
This is because the initiation of `TestHiveContext` is being failed due to trying to load non-existing data paths (test tables).

This is introduced from #14005

This enables the tests with SparkR.

## How was this patch tested?

Manually,

**Before** (on Mac OS)

```
...
Skipped ------------------------------------------------------------------------
1. create DataFrame from RDD (test_sparkSQL.R#200) - Hive is not build with SparkSQL, skipped
2. test HiveContext (test_sparkSQL.R#1041) - Hive is not build with SparkSQL, skipped
3. read/write ORC files (test_sparkSQL.R#1748) - Hive is not build with SparkSQL, skipped
4. enableHiveSupport on SparkSession (test_sparkSQL.R#2480) - Hive is not build with SparkSQL, skipped
5. sparkJars tag in SparkContext (test_Windows.R#21) - This test is only for Windows, skipped
...
```

**After** (on Mac OS)

```
...
Skipped ------------------------------------------------------------------------
1. sparkJars tag in SparkContext (test_Windows.R#21) - This test is only for Windows, skipped
...
```

Please refer the tests below (on Windows)
 - Before: https://ci.appveyor.com/project/HyukjinKwon/spark/build/45-test123
 - After: https://ci.appveyor.com/project/HyukjinKwon/spark/build/46-test123

Author: hyukjinkwon <[email protected]>

Closes #14889 from HyukjinKwon/SPARK-17326.

(cherry picked from commit 50bb142)
Signed-off-by: Shivaram Venkataraman <[email protected]>
@asfgit asfgit closed this in 50bb142 Aug 31, 2016
@HyukjinKwon HyukjinKwon deleted the SPARK-17326 branch January 2, 2018 03:44
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