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

Cannot run tests #19

Open
arilamstein opened this issue Apr 9, 2021 · 6 comments
Open

Cannot run tests #19

arilamstein opened this issue Apr 9, 2021 · 6 comments
Labels
check Necessary to pass R CMD CHECK

Comments

@arilamstein
Copy link
Contributor

#12 is turning out to be a much larger task that I realized. My current PR is turning into addressing a small subset of the problem (namely, use @importFrom <package> <function> for all functions to remove ambiguity about which functions we really want to use. From there we can presumably move most of the functions from Depends to Imports).

But I just realized a very serious issue that is probably worth an Issue of its own. R CMD CHECK is halting because it simply cannot run the package's test suite at all:

> test_check("tbltools")
Error in file(file, ifelse(append, "a", "w")) : 
  cannot open the connection
Calls: test_check ... write.csv -> eval.parent -> eval -> eval -> write.table -> file
In addition: Warning message:
In file(file, ifelse(append, "a", "w")) :
  cannot open file '/aim/users/ari/github/tbltools.Rcheck/tests/testthat/tests/testthat/helper-test_csv_data.csv': No such file or directory
Execution halted

I have not investigated this issue yet at all.

@arilamstein arilamstein added the check Necessary to pass R CMD CHECK label Apr 9, 2021
@dholstius
Copy link
Member

test() worked just fine for me now; so did test_local() and test_package("tbltools").

Can I assume the larger context is that check() is running test_check()?

I'd say maybe a filesystem permissions issue on the server, except the path starts with /aim/users/ari/github/, and I can't imagine why this would fail.

@arilamstein
Copy link
Contributor Author

@dholstius Thanks for looking into this. I just looked at this more closely and I see the error. If you look closely at the path it is looking for ('/aim/users/ari/github/tbltools.Rcheck/tests/testthat/tests/testthat/helper-test_csv_data.csv') I think you will see that it does not in fact exist.

The issue is that there is duplication in the file path it is using: tests/testthat/tests/testthat.

It took me a while to realize this as when I tried to find the file I could. But that's because the file is in /aim/users/ari/github/tbltools.Rcheck/tests/testthat.

I think that we need to figure this out. But I am going to prioritize it a bit lower than fixing the errors related to package dependency issues.

@dholstius
Copy link
Member

dholstius commented Apr 15, 2021

Ah. Maybe test_check() does not play well with here::here() (or no longer does?):

test_csv_file <-
here::here(
"tests",
"testthat",
"helper-test_csv_data.csv")

There may be a newer or better way to incorporate data into tests. Perhaps it'd be good to ask the testthat package/community, or read some of the newer testthat documentation.

Lower priority sounds good to me as long as you can successfully run test() prior to check().

@arilamstein
Copy link
Contributor Author

Got it. When I opened this I was only running the tests via R CMD CHECK and that's indeed the only path that's failing. Like you, I can run the tests the other ways you mention. According to the documentation, ?test_check is only run by R CMD CHECK.

Can you confirm that you are seeing that two tests currently fail? Also, can you confirm whether these failures predate my work on R CMD CHECK? (I.e. I'd like to make sure that my work hasn't caused these failures). The failures I see are:

══ Failed tests ═════════════════════════════════════════════════════════════════════════════
── Failure (test-write_csv.R:78:3): Inf signif read back ───────────────────────────
`csv_lines` not equivalent to c(...).
2/4 mismatches
x[2]: "\"bar\",\"2019-10-09\",\"1\",\"1.4700e+02\""
y[2]: "\"bar\",\"2019-10-09\",\"1\",\"147\""

x[4]: "\"bar\",\"2019-10-09\",\"3\",\"7.8125e-03\""
y[4]: "\"bar\",\"2019-10-09\",\"3\",\"0.0078125\""
── Failure (test-write_csv.R:105:3): default signif ────────────────────────────
`csv_lines` not equivalent to c(...).
2/4 mismatches
x[2]: "\"bar\",\"2019-10-09\",\"1\",\"1.4700e+02\""
y[2]: "\"bar\",\"2019-10-09\",\"1\",\"147\""

x[4]: "\"bar\",\"2019-10-09\",\"3\",\"7.8125e-03\""
y[4]: "\"bar\",\"2019-10-09\",\"3\",\"0.0078125\""

[ FAIL 2 | WARN 0 | SKIP 0 | PASS 81 ]
Error: Test failures

I think there are two things here:

  1. I'd like to finish the Depends -> Imports stuff. So if I can run the tests manually and confirm that my checkins don't add any additional failures, that's adequate for now.
  2. Longer term, I'd like to get test_check() to work again. That seems like part of the larger "fix R CMD CHECK" issue.

@arilamstein
Copy link
Contributor Author

Ah. Maybe test_check() does not play well with here::here() (or no longer does?):

test_csv_file <-
here::here(
"tests",
"testthat",
"helper-test_csv_data.csv")

There may be a newer or better way to incorporate data into tests. Perhaps it'd be good to ask the testthat package/community, or read some of the newer testthat documentation.

Lower priority sounds good to me as long as you can successfully run test() prior to check().

Also, specifically regarding your thought that "there may be a newer or better way to incorporate data into tests", the help files for test_package say:

There are two other types of special file that we no longer recommend using:

Helper files start with helper and are executed before tests are run. They're also loaded by devtools::load_all(), so there's no real point to them and you should just put your helper code in R/.

Teardown files start with teardown and are executed after the tests are run. Now we recommend interleave setup and cleanup code in setup- files, making it easier to check that you automatically clean up every mess that you make.

I mention this because the file you reference starts with helper.

@dholstius
Copy link
Member

Thanks! Yes, we should move away from the old helper-* style. And, the Depends → Imports work is priority. It's OK with me that those tests you quoted are failing. I'd like to fix those. But not adding more failures is good for now. Thanks for asking!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check Necessary to pass R CMD CHECK
Projects
None yet
Development

No branches or pull requests

2 participants