Skip to content
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
Merged

Conversation

jefshe
Copy link
Contributor

@jefshe 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
@eddelbuettel
Copy link
Owner

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
Copy link
Owner

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants