-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[MINOR][DOCS] Improve Running R Tests docs #18271
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
Conversation
|
Test build #77907 has finished for PR 18271 at commit
|
docs/building-spark.md
Outdated
|
|
||
| To run the SparkR tests you will need to install the R package `testthat` | ||
| (run `install.packages(testthat)` from R shell). You can run just the SparkR tests using | ||
| (run `install.packages("testthat")` from R shell). You can run just the SparkR tests using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind updating this contents to be consistent with the unit test part -
https://github.com/apache/spark/blob/7e0cd1d9b168286386f15e9b55988733476ae2bb/R/README.md#examples-unit-tests if it sounds making sense to you?
This also looks including specifying the mirror from where to download the package.
|
Test build #77911 has finished for PR 18271 at commit
|
|
From trying to run the tests last week, I think more packages are actually needed. The |
docs/building-spark.md
Outdated
| To run the SparkR tests you will need to install the R package `testthat` | ||
| (run `install.packages(testthat)` from R shell). You can run just the SparkR tests using | ||
| the command: | ||
| To run the SparkR tests you will need to install the [testthat](http://cran.r-project.org/web/packages/testthat/index.html) package first: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use https://cran.r-project.org/package=testthat instead
|
Test build #77977 has finished for PR 18271 at commit
|
R/README.md
Outdated
| ./bin/spark-submit examples/src/main/r/dataframe.R | ||
| ``` | ||
| You can also run the unit tests for SparkR by running. You need to install the [testthat](http://cran.r-project.org/web/packages/testthat/index.html) package first: | ||
| You can also run the unit tests for SparkR by running. You need to install the [knitr](https://cran.r-project.org/package=knitr), [rmarkdown](https://cran.r-project.org/package=rmarkdown), [testthat](https://cran.r-project.org/package=testthat), [e1071](https://cran.r-project.org/package=e1071) and [survival](https://cran.r-project.org/package=survival) packages first: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than duplicate this information, I wonder if you can just link to http://spark.apache.org/docs/latest/building-spark.html#running-r-tests here ?
|
Test build #77995 has finished for PR 18271 at commit
|
R/README.md
Outdated
| R -e 'install.packages("testthat", repos="http://cran.us.r-project.org")' | ||
| ./R/run-tests.sh | ||
| ``` | ||
| You can run the unit tests following [running-r-tests](http://spark.apache.org/docs/latest/building-spark.html#running-r-tests). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will look funny; you want something like "You can run R unit tests by following the instructions under Running R Tests"
docs/building-spark.md
Outdated
| the command: | ||
| To run the SparkR tests you will need to install the [knitr](https://cran.r-project.org/package=knitr), [rmarkdown](https://cran.r-project.org/package=rmarkdown), [testthat](https://cran.r-project.org/package=testthat), [e1071](https://cran.r-project.org/package=e1071) and [survival](https://cran.r-project.org/package=survival) packages first: | ||
|
|
||
| R -e 'install.packages(c("knitr", "rmarkdown", "testthat", "e1071", "survival"), repos="http://cran.us.r-project.org")' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about a more global mirror? (e.g. https://cloud.r-project.org/)
…er Running R Tests"
| R -e 'install.packages("testthat", repos="http://cran.us.r-project.org")' | ||
| ./R/run-tests.sh | ||
| ``` | ||
| You can run R unit tests by following the instructions under [Running R Tests](http://spark.apache.org/docs/latest/building-spark.html#running-r-tests). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry, @wangyum, would you mind update https://github.com/wangyum/spark/blob/0da2f5b5b3ac122a2ba5b295b4ae8c4f4af02758/R/WINDOWS.md?
|
Test build #78013 has finished for PR 18271 at commit
|
|
For me, it looks good. I assume we consider #18271 (comment) but it looks fine as is too. |
|
Test build #78021 has finished for PR 18271 at commit
|
R/WINDOWS.md
Outdated
| 4. Set the environment variable `HADOOP_HOME` to the full path to the newly created `hadoop` directory. | ||
|
|
||
| 5. Run unit tests for SparkR by running the command below. You need to install the [testthat](http://cran.r-project.org/web/packages/testthat/index.html) package first: | ||
| 5. Run unit tests for SparkR by running the command below. You need to install the [knitr](https://cran.r-project.org/package=knitr), [rmarkdown](https://cran.r-project.org/package=rmarkdown), [testthat](https://cran.r-project.org/package=testthat), [e1071](https://cran.r-project.org/package=e1071) and [survival](https://cran.r-project.org/package=survival) packages first: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, but again why duplicate this information? are there more locations this is repeated? we should just point to one location where these details are listed, to make sure they don't get out of sync
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows is a little diffrence from Linux, so only link dependency package.
Linux:
R -e 'install.packages(c("knitr", "rmarkdown", "testthat", "e1071", "survival"), repos="http://cran.us.r-project.org")'
Windows:
R -e "install.packages(c('knitr', 'rmarkdown', 'testthat', 'e1071', 'survival'), repos='http://cran.us.r-project.org')"
|
Test build #78048 has finished for PR 18271 at commit
|
|
+1 for not duplicating.
You can change the command line to use " to make it work in both platforms
|
|
Test build #78096 has finished for PR 18271 at commit
|
|
Removed the duplicating. It works both for Windows and linux: |
|
Merged to master/2.2 |
## What changes were proposed in this pull request?
Update Running R Tests dependence packages to:
```bash
R -e "install.packages(c('knitr', 'rmarkdown', 'testthat', 'e1071', 'survival'), repos='http://cran.us.r-project.org')"
```
## How was this patch tested?
manual tests
Author: Yuming Wang <[email protected]>
Closes #18271 from wangyum/building-spark.
(cherry picked from commit 45824fb)
Signed-off-by: Sean Owen <[email protected]>
What changes were proposed in this pull request?
Update Running R Tests dependence packages to:
R -e "install.packages(c('knitr', 'rmarkdown', 'testthat', 'e1071', 'survival'), repos='http://cran.us.r-project.org')"How was this patch tested?
manual tests