Bug 17143 - R CMD check finds only tests with uppercase .R
Summary: R CMD check finds only tests with uppercase .R
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: Misc (show other bugs)
Version: R-devel (trunk)
Hardware: All All
: P5 minor
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2016-09-07 12:22 UTC by flying-sheep
Modified: 2016-09-12 07:27 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description flying-sheep 2016-09-07 12:22:55 UTC
Hi,

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 flying-sheep 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 flying-sheep 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 flying-sheep 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?