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

Add tests #14

Closed
jranke opened this issue Nov 24, 2020 · 9 comments · Fixed by #15
Closed

Add tests #14

jranke opened this issue Nov 24, 2020 · 9 comments · Fixed by #15

Comments

@jranke
Copy link
Contributor

jranke commented Nov 24, 2020

Currently there are no tests driven from tests/ either directly or via a test runner in the package but only tests relying on help page examples that do not check the output of the compiled functions.

@jranke
Copy link
Contributor Author

jranke commented Nov 24, 2020

Thanks, that's more precise now.

@eddelbuettel
Copy link
Owner

The advantage of tinytest is that it is very easy (and always possible) the run the tests of an installed or source package. I actually do that a lot (often via tt.r which is a convenience wrapper for the shell) --- but I also appreciate the fact that the package itself is zero-depends. A trade-off.

@jranke
Copy link
Contributor Author

jranke commented Nov 24, 2020

You decide, if you like tinytest I will give a go.

@eddelbuettel
Copy link
Owner

It's worth a try. It is purely additive: add a Suggests:, copy the caller into tests/ and add a file. See how it goes. Be incremental.

@jranke
Copy link
Contributor Author

jranke commented Nov 24, 2020

BTW, the test fails in the same way when run in the CRAN version 0.3.16.

It's enough to put the following code in the tests/ directory:

library(inline)

n = 10L
x = 1:10

## A simple Fortran example - n and x: assumed-size vector
code <- "
      integer i
      do 1 i=1, n(1)
    1 x(i) = x(i)**3
"

cubefn <- cfunction(signature(n = "integer", x = "numeric"), code,
   convention = ".Fortran")

and I get

* checking tests ...
  Running ‘cfunction.R’
 ERROR
Running the tests in ‘tests/cfunction.R’ failed.
Last 13 lines of output:
    4:       DOUBLE PRECISION x(*)
    5: 
    6:       integer i
    7:       do 1 i=1, n(1)
    8:     1 x(i) = x(i)**3
    9: 
   10:       RETURN
   11:       END
   12: 
  
  Compilation ERROR, function(s)/method(s) not created!
  Error in compileCode(f, code, language, verbose) : 
    Error in file(filename, "r", encoding = encoding) :   cannot open the connectionCalls: local ... eval.parent -> eval -> eval -> eval -> eval -> source -> fileIn addition: Warning message:In file(filename, "r", encoding = encoding) :  cannot open file 'startup.Rs': No such file or directoryExecution halted
  Calls: cfunction -> compileCode
  Execution halted
* checking PDF version of manual ... OK
* DONE

Status: 1 ERROR

As mentioned above, this is with the released version.

@eddelbuettel
Copy link
Owner

eddelbuettel commented Nov 24, 2020

🤷‍♂️

I really think it's a known (to calls of R CMD check) issue I have seen before / we all have seen before but I don't have time right now, unfortunately, to google or research this for you. Of course I could also be flat out wrong. Who knows?

Back to real work, and end to the staccato over this topic. Later...

@eddelbuettel
Copy link
Owner

eddelbuettel commented Nov 25, 2020

Thanks for the PR. I should have done that a while ago but because some low-dependency packages (such as Rcpp) also have a Suggests: inline` I was overly hesitant.

I also just increased the version number to signal this 'in-between releases' status and made a fairly routine update to the CI script I use (and similarly update in a few other places). It would be great if you could rebase your other branch to the main one here.

Edit: And I was of course too optimistic as switching to a matrix build with macOS promptly goes belly-up over Fortran. I'll disable the macOS part.

@jranke
Copy link
Contributor Author

jranke commented Nov 25, 2020

You're welcome. My pleasure!

I don't have any macOS so I can't do anything there.

I rebased my other branch as suggested and will add tests for the new stuff there... Having tests will really make it easier to see if I goof up while trying to fix things! The tests already relevaled two things I overlooked (one was in SetCMethod which I hadn't looked at). I currently do not plan to add tests for cxxfunction. I saw it uses compileCode() from R/cfunction.R but I think I don't need to touch that.

@eddelbuettel
Copy link
Owner

No macOS here either, and that is the part that Travis CI will make us pay so off it is. [ I am currently reworking / rebranding my Travis setup script as it works unchanged at Travis, GitHub Actions, Azure Pipelines, ... so we could always run it somewhere else. ]

And yes, having tests is good. I'll add something simply for cxxfunction 'in due course'. No rush. The "mutex" is yours, make your changes; and I appreciate that you are being careful and diligent about it. And in the process don't forget to add yourself in DESCRIPTION as an author.

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 a pull request may close this issue.

2 participants