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

Make PR#48 actually work #50

Merged
merged 1 commit into from Sep 6, 2018

Conversation

Projects
None yet
2 participants
@jefshe
Contributor

jefshe commented Sep 6, 2018

So it turns out my last pull request didn't fix the issue and the unit test I added fails locally.

Sorry about that, do the unit tests not get run on travis?

@eddelbuettel eddelbuettel merged commit beb6f08 into eddelbuettel:master Sep 6, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eddelbuettel

This comment has been minimized.

Owner

eddelbuettel commented Sep 6, 2018

Strange. The unit tests run under R CMD check. it should also have come up at your end if you did R CMD build ... followed by R CMD check ....

@eddelbuettel

This comment has been minimized.

Owner

eddelbuettel commented Sep 6, 2018

And it does fail locally here:

[...]
✔  checking for unstated dependencies in ‘tests’
─  checking tests ...  Running ‘runUnitTests.R’
    ERROR
   Running the tests in ‘tests/runUnitTests.R’ failed.
   Last 13 lines of output:
     Error in attributes(xobj) <- attrib[names(attrib) != "class"] : 
       'names' attribute [11] must be the same length as the vector [1]
     test.serialize.sublist: (1 checks) ... OK (0 seconds)
     test.serialize_pb: (2 checks) ... OK (0.01 seconds)
     test.serialize_pb.alldatasets: (1 checks) ... OK (0.17 seconds)
     > 
     > ## Return success or failure to R CMD CHECK
     > if (getErrors(tests)$nFail > 0) {
     +    stop("TEST FAILED!")
     + }
     > if (getErrors(tests)$nErr > 0) {
     +    stop("TEST HAD ERRORS!")
     + }
     Error: TEST HAD ERRORS!
     Execution halted
✔  checking for unstated dependencies in vignettes
✔  checking package vignettes in ‘inst/doc’
[...]

(using Gabor's rcmdcheck from the littler script rcc.r I use for this).

And it passes with your patch. Very strange that it did not trip Travis CI.

(We had some recent 'red' there I upgraded the underlying script to use R 3.5.* by defaul but the external repo I use for the protocol buffer libraries still ties us to R 3.4.*. I will change that here by switching to testing in Docker as I have done in a few other repos requiring an external and not-so-common library.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment