Skip to content

Conversation

@felixcheung
Copy link
Member

@felixcheung felixcheung commented Dec 9, 2016

What changes were proposed in this pull request?

Several SparkR API calling into JVM methods that have void return values are getting printed out, especially when running in a REPL or IDE.
example:

> setLogLevel("WARN")
NULL

We should fix this to make the result more clear.

Also found a small change to return value of dropTempView in 2.1 - adding doc and test for it.

How was this patch tested?

manually - I didn't find a expect_*() method in testthat for this

@SparkQA
Copy link

SparkQA commented Dec 9, 2016

Test build #69933 has finished for PR 16237 at commit 75f56c8.

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

@shivaram
Copy link
Contributor

shivaram commented Dec 9, 2016

Does expert_silent do what we want here ? r-lib/testthat#261

@shivaram
Copy link
Contributor

shivaram commented Dec 9, 2016

Actually looks like that doesn't do what we want - that checks for outputs on stdout, warnings or errors etc.

expect_equal(count(tables), 2)
suppressWarnings(dropTempTable("table1"))
dropTempView("table2")
expect_true(dropTempView("table2"))
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a bunch of other places where we call dropTempView - Might be good to add this to those places which we think are testing temp views ?

@shivaram
Copy link
Contributor

LGTM. Will wait for Jenkins, AppVeyor

@SparkQA
Copy link

SparkQA commented Dec 10, 2016

Test build #69945 has finished for PR 16237 at commit 3b3c58d.

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

@shivaram
Copy link
Contributor

Merging into master, branch-2.1

asfgit pushed a commit that referenced this pull request Dec 10, 2016
…ethods with void return values

## What changes were proposed in this pull request?

Several SparkR API calling into JVM methods that have void return values are getting printed out, especially when running in a REPL or IDE.
example:
```
> setLogLevel("WARN")
NULL
```
We should fix this to make the result more clear.

Also found a small change to return value of dropTempView in 2.1 - adding doc and test for it.

## How was this patch tested?

manually - I didn't find a expect_*() method in testthat for this

Author: Felix Cheung <felixcheung_m@hotmail.com>

Closes #16237 from felixcheung/rinvis.

(cherry picked from commit 3e11d5b)
Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
@asfgit asfgit closed this in 3e11d5b Dec 10, 2016
robert3005 pushed a commit to palantir/spark that referenced this pull request Dec 15, 2016
…ethods with void return values

## What changes were proposed in this pull request?

Several SparkR API calling into JVM methods that have void return values are getting printed out, especially when running in a REPL or IDE.
example:
```
> setLogLevel("WARN")
NULL
```
We should fix this to make the result more clear.

Also found a small change to return value of dropTempView in 2.1 - adding doc and test for it.

## How was this patch tested?

manually - I didn't find a expect_*() method in testthat for this

Author: Felix Cheung <felixcheung_m@hotmail.com>

Closes apache#16237 from felixcheung/rinvis.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…ethods with void return values

## What changes were proposed in this pull request?

Several SparkR API calling into JVM methods that have void return values are getting printed out, especially when running in a REPL or IDE.
example:
```
> setLogLevel("WARN")
NULL
```
We should fix this to make the result more clear.

Also found a small change to return value of dropTempView in 2.1 - adding doc and test for it.

## How was this patch tested?

manually - I didn't find a expect_*() method in testthat for this

Author: Felix Cheung <felixcheung_m@hotmail.com>

Closes apache#16237 from felixcheung/rinvis.
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