Bug 17143

Summary: R CMD check finds only tests with uppercase .R
Product: R Reporter: Philipp Angerer <flying-sheep>
Component: MiscAssignee: R-core <R-core>
Status: CLOSED FIXED    
Severity: minor CC: maechler, murdoch
Priority: P5    
Version: R-devel (trunk)   
Hardware: All   
OS: All   

Description Philipp Angerer 2016-09-07 12:22:55 UTC

this one is pretty invisible and therefore annoying since passing tests only differ from no tests in two more lines in the log.

R CMD check should also find files in tests/ that end in .r, not only .R

offending lines of code: https://github.com/wch/r-source/blob/e5b21d0397c607883ff25cca379687b86933d730/src/library/tools/R/check.R#L379-L381
Comment 1 Duncan Murdoch 2016-09-07 13:23:20 UTC
This is how it is documented in Writing R Extensions, but it does make sense either to accept .r (and .rin?) files, or to warn about their presence.
Comment 2 Martin Maechler 2016-09-08 06:55:05 UTC
(In reply to Duncan Murdoch from comment #1)
> This is how it is documented in Writing R Extensions, but it does make sense
> either to accept .r (and .rin?) files, or to warn about their presence.

In this case, I'd like we follow the documentation.  <foo>.r is so ugly and the capitalized  *.R was indeed very much the purpose when invented it.

OTOH, I would not know how many packages currently use the wrong capitalization, and if we, notably the CRAN team was willing to put up with too many new warnings.
Comment 3 Duncan Murdoch 2016-09-08 07:24:10 UTC
We are inconsistent about which extensions are accepted:  the R subdir is most lenient, accepting .R, .S, .q, .r, or .s, demo and data accept .R or .r.  It's not R code, but man accepts .Rd or .rd.  

I would certainly object to making any of those more strict.  It's probably less work to accept test/*.r than it would be to issue warnings about it (though the OP's identified lines aren't the only ones that would need changing).
Comment 4 Philipp Angerer 2016-09-10 17:48:37 UTC
the problem is that if someone chooses to use .r, it’ll work everywhere but test files with that extension will simply silently fail to run.

if anyone decides to keep it like it is, .r files should be included in a R CMD check WARNING that says there were skipped files in the /tests/ directory.
Comment 5 Philipp Angerer 2016-09-10 17:51:28 UTC
> It's probably less work to accept test/*.r

Certainly. I linked the line for the change, and fixing it involves the addition of exactly 2 characters: “|r”
Comment 6 Duncan Murdoch 2016-09-11 13:05:02 UTC
Comment 5 (and the original description) are false.  Changes are needed in several other places to allow .r test files.  I've now made these changes in R-devel r71233.
Comment 7 Philipp Angerer 2016-09-12 07:27:42 UTC
sorry for underestimating the amount of work it took! i wonder however:

.check_packages is described as “The main function for "R CMD check"”
.runPackageTests is described as “used by R CMD check”
testInstalledPackage also exists

and all three glob for tests/*.R and friends. shouldn’t they all just call the same function?