PRs pass tic but not R CMD Check

[lorenzwalthert]

I note that various PRs (e.g. #183, not merged or #179, merged) that do not pass R CMD Check on travis, but are showed as passing on the conversation tab of the PR.
@krlmlr Is that an issue with tic?
I submitted #185 to fix R CMD Check. The error was arguably introduced with #174.

[krlmlr]

This may be a change in rcmdcheck, tic expects a named list with components errors, warnings and notes as a result of rcmdcheck::rcmdcheck_(). @gaborcsardi: Can you please confirm?

[gaborcsardi]

Those components are still there, but there are some new ones as well.

[lorenzwalthert]

I merged #185, so the master branch R CMD Check is perfect again, but we can rebuild previous PRs such as #179 that suffer from the same problem.

[gaborcsardi]

OK. (I don't understand, though, #185 is only adding a single space, seemingly.)

[lorenzwalthert]

Yes, I only added a space, but this space is needed for the tests to pass. Most tests in styler are "take a file, style it and compare it to a reference file". So I updated the reference file because before, the styling rule was adapted and yields a different outcome than before.

[gaborcsardi]

Oh, so this is not an rcmdcheck issue then.

[lorenzwalthert]

@krlmlr so should we file an issue in tic and close this?

[krlmlr]

@gaborcsardi: I can replicate locally.

git clone https://github.com/krlmlr/tic.package.git --branch f-test-failure
cd tic.package
R -q -e 'remotes::install_github("ropenscilabs/tic")'
R -q -e 'install.packages("rcmdcheck")'
R -q -e 'tic::script()' # fails to fail
R -q -e 'remotes::install_github("r-lib/rcmdcheck")'
R -q -e 'tic::script()' # fails successfully

How do I reliably count the errors, warnings, and notes from an rcmdcheck object in both the CRAN and GitHub versions of rcmdcheck?

[gaborcsardi]

I don't understand. So this is an rcmdcheck issue, then? How are you doing it now? This should do it:

length(x$errors) + length(x$warnings) + length(x$notes)

[krlmlr]

Relevant code in tic looks similar: https://github.com/ropenscilabs/tic/blob/master/R/steps-rcmdcheck.R#L13-L22

It seems to work as expected with the GitHub version of rcmdcheck, but not with the CRAN version of rcmdcheck, this is what my reprex above demonstrates.

[gaborcsardi]

This seems to be a bug in the CRAN version of rcmdcheck. The error is not even found.

[lorenzwalthert]

Am I right that the next rcmdcheck CRAN release is going fix this so we can close this issue?

[krlmlr]

tic will install rcmdcheck from GitHub for now, please watch out for check failures that are not recognized by Travis or AppVeyor.

[lorenzwalthert]

WIth #205, I get perfect local rcmdcheck::rcmdcheck(), one NOTE with devtools::test() because of a top level file in the directoy (which is not checked in git) and an early terminated CI using tic.

[krlmlr]

NOTEs currently don't cause a test to fail, but this can be changed in tic.R. Should we?

[lorenzwalthert]

Well since the offending file causing devtools::check() (and not dectools::test() as stated above) is not checked into git, the note is not the problem on our CI (it's rather that rcmdcheck::rcmdcheck() should give me a NOTE locally). My main point, however, was that as with #205, I don't see a X ERRORS, Y WARNINGS, Z NOTES at the end of the process (which I expect).

[krlmlr]

Yeah, tic should print a summary.