Skip to content

[SPARK-16577][SPARKR] Add CRAN documentation checks to run-tests.sh #14759

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.

Sign up for GitHub

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

Closed
shivaram wants to merge 7 commits into from
Closed

[SPARK-16577][SPARKR] Add CRAN documentation checks to run-tests.sh #14759

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
349d95d
Add CRAN documentation checks to run-tests.sh
shivaram Aug 21, 2016
61f4584
Merge branch 'master' of https://github.com/apache/spark into sparkr-…
shivaram Aug 22, 2016
96216ac
Remove e1071 to make Jenkins work for now
shivaram Aug 22, 2016
9de5e59
Revert change to DESCRIPTION
shivaram Aug 22, 2016
5df8936
ALso check for errors, notes. Refactor options
shivaram Aug 22, 2016
2bde2b4
Handle extra NOTE in Jenkins
shivaram Aug 22, 2016
e040455
Merge branch 'master' of https://github.com/apache/spark into sparkr-…
shivaram Aug 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion R/check-cran.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ $FWDIR/create-docs.sh

VERSION=`grep Version $FWDIR/pkg/DESCRIPTION | awk '{print $NF}'`

"$R_SCRIPT_PATH/"R CMD check --as-cran SparkR_"$VERSION".tar.gz
"$R_SCRIPT_PATH/"R CMD check $CRAN_CHECK_OPTIONS --as-cran SparkR_"$VERSION".tar.gz

popd > /dev/null
1 change: 0 additions & 1 deletion R/pkg/DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Depends:
methods
Suggests:
testthat,
e1071,
survival
Description: The SparkR package provides an R frontend for Apache Spark.
License: Apache License (== 2.0)
Expand Down
22 changes: 19 additions & 3 deletions R/run-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,30 @@ rm -f $LOGFILE
SPARK_TESTING=1 $FWDIR/../bin/spark-submit --driver-java-options "-Dlog4j.configuration=file:$FWDIR/log4j.properties" --conf spark.hadoop.fs.default.name="file:///" $FWDIR/pkg/tests/run-all.R 2>&1 | tee -a $LOGFILE
FAILED=$((PIPESTATUS[0]||$FAILED))

# Also run the documentation tests for CRAN
CRAN_CHECK_LOG_FILE=$FWDIR/cran-check.out
rm -f $CRAN_CHECK_LOG_FILE
CRAN_CHECK_OPTIONS="--no-tests --no-manual" $FWDIR/check-cran.sh 2>&1 | tee -a $CRAN_CHECK_LOG_FILE
Copy link
Member

@felixcheung felixcheung Aug 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I think as the next step should generate the manual as a part of this test too? Agree not critical for now.

Copy link
Contributor Author

@shivaram shivaram Aug 22, 2016

Choose a reason for hiding this comment

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

Yeah I've turned it off because of the lack of latex packages on Jenkins. I dont think this matters for now as the PDF manual isn't something we use. But we can revisit this later if required.

Copy link
Member

@felixcheung felixcheung Aug 22, 2016

Choose a reason for hiding this comment

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

Right. I think pdf is generally available for each package posted on CRAN?

Copy link
Contributor Author

@shivaram shivaram Aug 22, 2016

Choose a reason for hiding this comment

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

Yeah - I think it will get generated by CRAN online (we submit a source package to them I think) - We will also do a check before we release a package.

FAILED=$((PIPESTATUS[0]||$FAILED))

HAS_CRAN_WARNING="$(grep -c WARNING $CRAN_CHECK_LOG_FILE)"
Copy link
Member

@felixcheung felixcheung Aug 22, 2016

Choose a reason for hiding this comment

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

should check for ERROR or NOTE too?

For ERROR, I guess that would go to exit code?
For NOTE, I think we should probably compare the note count. Perhaps then we just look for the line Status: 2 NOTEs at the end of the output?

Copy link
Contributor Author

@shivaram shivaram Aug 22, 2016

Choose a reason for hiding this comment

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

Good point - Added that.


if [[ $FAILED != 0 ]]; then
cat $LOGFILE
echo -en "\033[31m" # Red
echo "Had test failures; see logs."
echo -en "\033[0m" # No color
exit -1
else
echo -en "\033[32m" # Green
echo "Tests passed."
echo -en "\033[0m" # No color
if [[ $HAS_CRAN_WARNING != 0 ]]; then
cat $CRAN_CHECK_LOG_FILE
echo -en "\033[31m" # Red
echo "Had CRAN check warnings; see logs."
echo -en "\033[0m" # No color
exit -1
else
echo -en "\033[32m" # Green
echo "Tests passed."
echo -en "\033[0m" # No color
fi
fi