ropensci / RNeXML

Implementing semantically rich NeXML I/O in R
https://docs.ropensci.org/RNeXML
Other
13 stars 9 forks source link

taxize_nexml tests are inconsistently in part run and in part skipped on CRAN #177

Closed hlapp closed 5 years ago

hlapp commented 5 years ago

The tests defined in tests/testthat/test_taxonomy.R are all apparently skipped on CRAN (any particular reason for that? skipping tests seems alway a fraught idea.).

However, the setup code preceding the test cases already runs taxize_nexml(), and presumably does run on CRAN as well. (It also suffers from the same problem reported in #176.)

cboettig commented 5 years ago

On CRAN, network-related tasks will sometimes succeed but can also timeout sporadically, so we tend try and minimize the number of network-intensive tests CRAN has to run so we don't get kicked off. I agree that skipping tests fraught, but note these tests are not skipped on travis, and they are not skipped in local testing. (ropensci also tickles travis builds to make sure tests run even when no one is committing to repositories).

Actually, I think CRAN is pretty unique among software archives in that in runs the full test suite of every package every night (across 12 different architectures! https://cran.r-project.org/web/checks/check_results_RNeXML.html). What other distribution services even do that?

hlapp commented 5 years ago

Ah yes, they aren't skipped on Travis, my mistake. What about the taxize_nexml() prior to the tests,, though? It's harmless because it can't time out a test?

hlapp commented 5 years ago

Ah yes, they aren't skipped on Travis, my mistake.

Actually I take that back. The tests on Travis are run with parameter --as-cran. Doesn't that mean that tests skipped on CRAN get skipped on Travis as well?

cboettig commented 5 years ago

no, because skip_on_cran() thought about that. --as-cran is a parameter to that invokes additional checks on R CMD check, and yeah, we almost always want to run with those, including on Travis. See ?skip_on_cran() which looks for the environmental variable NOT_CRAN being set. devtools sets this, so checks on Travis, Appveyor, and locally are run with NOT_CRAN being set, but clearly CRAN doesn't set this, and thus checks are skipped.

Of course if I had been careful to skip_on_cran() all of the taxize_nexml() calls, then we'd be able to confirm these are indeed being run on travis by checking code coverage: https://codecov.io/gh/ropensci/RNeXML/src/master/R/taxize_nexml.R . But it sounds like we run that at least once without a skip_on_cran().

hlapp commented 5 years ago

Looks like there maybe isn't any real issue here, aside from _the setup code preceding the test cases already runs taxize_nexml(), and presumably does run on CRAN as well._ Sounds like this doesn't concern you, so I'm going to close this.