dpc10ster / RJafroc

Artificial Intelligence: Evaluating AI, optimizing AI
19 stars 8 forks source link

Behavior of expect_known_output and using it with Git #18

Closed dpc10ster closed 5 years ago

dpc10ster commented 5 years ago

Sorry to raise this issue at this late hour ...

I am not sure that I am using this function correctly.

In looking at the testthat package code it is clear that, on the first call, it checks if the temporary file already exists and if so it does not overwrite it. That would be a legitimate test as then one would be comparing newly generated output with previously saved "good output".

When run on Travis, the temporary file does not exist, so the code generates output and writes to the file and then compares the contents of the file to newly generated values, which are guaranteed to be identical. If so the test is meaningless.

The documentation of testthat states:

These expectations should be used in conjunction with git, as otherwise there is no way to revert to previous values. Git is particularly useful in conjunction with expect_known_output() as the diffs will show you exactly what has changed.

This implies the temporary file should be under Git control - which is definitely not the case on my machine. For example, a typical value for temple() is:

> tempfile() 
[1] "/var/folders/d1/mx6dcbzx3v39r260458z2b200000gn/T//Rtmp7poTqc/file8ba9775dd8fe" 
dpc10ster commented 5 years ago

Here is an extract from the source file; I am still trying to figure this out.

' These expectations should be used in conjunction with git, as otherwise

' there is no way to revert to previous values. Git is particularly useful

' in conjunction with expect_known_output() as the diffs will show you

' exactly what has changed.

'

' Note that known values updates will only be updated when running tests

' interactively. R CMD check clones the package source so any changes to

' the reference files will occur in a temporary directory, and will not be

' synchronised back to the source package.

'

' @export

' @param file File path where known value/output will be stored.

' @param update Should the file be updated? Defaults to TRUE, with

' the expectation that you'll notice changes because of the first failure,

' and then see the modified files in git.

pwep commented 5 years ago

A call to expect_known_output() will do one of two things, depending on the named file.

The named file is often a temporary file, the unique filename being created from tempfile().

When run on Travis, the temporary file does not exist, so the code generates output and writes to the file and then compares the contents of the file to newly generated values, which are guaranteed to be identical. If so the test is meaningless.

On any system (Travis or otherwise), the temporary file will not exist in at the first call (so gets created, with the accompanying "Creating reference output" warning). It is a test of repeatability; does a function called twice with the same input generate the same output. The temporary file is a way of storing that output until the second call of expect_known_output() with that filename.

Git comes into play when you get an error. If there is a difference in outputs, you need to be able to open the files, or look for changes.

pwep commented 5 years ago

While not quote a test of last resort, expect_known_output() is a quick method of comparing potentially complicated output structures by saving them to a file. It is possible to compare the generated output to a reference output from the inst directory structure, however different platforms may save files in a slightly different way (platform specific newline character(s) would be an obvious example).

dpc10ster commented 5 years ago

I believe this is resolved; all goodValues are on git now and comparisons are made between these and newly generated values