Update to testthat will cause a spurious failure

[markmfredrickson]

I got the following from Hadley Wickam (an automated message). Basically we just need to replace a test string with the complete error message, not just the first part of the message.

Hi,

This is an automated email to let you know about the upcoming release of testthat, which will be submitted to CRAN on April 4. To check for potential problems, I ran R CMD check on your package optmatch (0.9-5).

I found: 1 error | 0 warnings | 0 notes.

checking tests ... ERROR
Running the tests in ‘tests/testthat-tests.R’ failed.
Last 13 lines of output:
      to be the same as your original data.

  Failed ---------------------------------------------------------------------
  1. Failure: Invalid mean.controls input

(@test.fullmatch.infeasible.recovery.R#15) error$message does not match "omit.fraction and mean.controls cannot both be specified.". Actual value: "omit.fraction and mean.controls cannot both be specified"

  DONE =======================================================================
  Error: Test failures
  In addition: Warning message:
  Placing tests in `inst/tests/` is deprecated. Please use

tests/testthat/ instead Execution halted

If I got an ERROR because I couldn't install your package (or one of it's dependencies), my apologies. You'll need to run the checks yourself (unfortunately I don't have the time to diagnose installation failures).

Otherwise, please carefully look at the results, and let me know if I've introduced a bug in teststhat. There are two common classes of new failures caused by this version of testthat:

1. expect_output() no longer automatically prints the object. You'll need to explicitly print() if you want to check the output of a print method.
2. expect_error() now only compares against the actual error message, not the prefix, e.g. "Error in foo(): ". You might need to change you match message to take this into account.

[josherrickson]

It's actually a bit simpler than that. It looks like the current version of testthat has a bug. Basically, the error string being searched for ends with a period. The actual error returned by fullmatch doesn't. The current version of testthat doesn't catch it. The new version does. I pushed up a fix.

Of note, this new version of testthat throws a "W" whenever it hits an unexpected warning. We get, unsurprisingly, over 100. I don't immediately see a way to disable that; perhaps this is a wakeup call to reign in the number of warnings we return? (Note this does not affect check, only test.)

[benthestatistician]

1. If I understand the issue of the "W"s being thrown, it won't arise during the CRAN checks, only during local testing. If this is correct, then I wouldn't put getting rid of the Ws on par with any of the other issues we have in the queue.
2. Perhaps the CRAN maintainers would waive through a new optmatch version if we justified it in terms along the lines @josherrickson lays out in his last comment. Josh, I know you've been reviewing Wickham's R Packages -- would you be willing to survey what remains between the current state of the package and having it ready for submission? For starters I see that we'd need to update NEWS and draft a letter to the cran maintenance team.

[josherrickson]

Open issues aside, I see nothing holding us back (code-wise) from pushing a new version up. All checks/build pass without incidence. We do get one warning when building on windows,

* checking CRAN incoming feasibility ... WARNING

Possibly mis-spelled words in DESCRIPTION:
  GLM (6:63)
  Mahalanobis (7:49)
  covariate (5:5)
  fullmatch (8:61)

The NEWS file did not appear to be updated for 0.9-5. Looking through closed issues since then, the large ones are #30 and #57.

[josherrickson]

I've been slowly going through the various warnings in the test files and cleaning them up over the last week or so, and b0e2894 contains my results. With the latest version of testthat from github (updated today, 3/29/16, via devtools::install_github("hadley/testthat"), all tests pass with no warnings, and all deprecated testthat functions have been updated, so we should be 100% for the new push of testthat to CRAN.

The spelling issues in my last comment have been addressed offline, and Ben has updated the NEWS file. make check and CRAN's win-builder both pass us with no notes or warnings. (win-builder likely uses the older version of testthat that is currently on CRAN, so we pass all tests there too.)

While we still haven't pushed the next version up, the concerns in this issue have been addressed.