[datatable-help] Streamlining unit tests with test_that

Fri Dec 31 16:33:26 CET 2010

I vote to move the tests all over to the new format. I also prefer the
expect_identical() syntax. That lets us do away with the test numbers.

- Tom

On Thu, Dec 23, 2010 at 4:59 PM, Matthew Dowle <mdowle at mdowle.plus.com> wrote:
> Hi Steve,
>
> Sorry not had time to look at this yet. Hoping to soon. Let me know if
> you're held up. It's high priority my side too as I'd like to fix
> the .Depends issue, the 2 outstanding bugs, get cedta sorted and release
> those few things onto CRAN soon. S4 tests would be nice to include.
>
> One thought though .. how would we know the ported tests correctly fail
> when they should?  Or perhaps it's easier to ask ... how would we know
> the ported tests fail in the same conditions the old tests fail?
>
> So I'm thinking we should keep the old test.data.table() there running
> even if it's just locked down. We could put new tests into test_that
> going forward (starting with S4) and that would save you the time of
> porting. Would that work?
>
> Matthew
>
>
> On Fri, 2010-12-17 at 22:50 -0500, Steve Lianoglou wrote:
>> Hi all,
>>
>> After a long delay, I've gotten around to testing some of the
>> test_that-ized unit tests and everything seems clean using Tom's
>> changes to cedta.R.
>>
>> Here are what my S4 tests are like:
>> https://github.com/lianos/datatable/blob/testthat/pkg/inst/tests/test-S4.R
>>
>> These run clean and don't have the problem with "locked environments"
>> and stuff that the "normal tests" have that are being run through a
>> man/*.Rd file.
>>
>> To give you an idea of what some of the current tests (1-16) look like
>> when they are test_that-ized, look here:
>> https://github.com/lianos/datatable/blob/testthat/pkg/inst/tests/test-all.R
>>
>> They can be even simpler to write if you don't insist on keeping track
>> of their test IDs (numbers). For instance, test 1 can go from this:
>>
>>   expect_that(TESTDT[SJ(4,6),v,mult="first"], is_identical_to(3L), info="1")
>>
>> to this:
>>
>>   expect_identical(TESTDT[SJ(4,6),v,mult="first"], 3L)
>>
>> Running tests are very simple as well. You can run them in "the usual
>> way" via R CMD check. When you do so, the results look like so:
>>
>> $ R CMD check <data.table>
>> ...
>> ... R CMD check progress ...
>> ** running tests for arch 'i386'
>>   Running ?test-all.R?
>>  OK
>> ** running tests for arch 'x86_64'
>>   Running ?test-all.R?
>>  OK
>> ...
>>
>> I deliberately put in a wrong expected result to show you what it
>> looks like when a test like this fails:
>>
>> $ R CMD check <data.table>
>> ...
>> ... R CMD check progress ...
>> ** running tests for arch 'i386'
>>   Running ?test-all.R?
>>  ERROR
>> Running the tests in 'tests/test-all.R' failed.
>> Last 13 lines of output:
>>
>>   S4-isms : .....
>>   Sorted Joins : 1...............
>>
>>   1. Failure: basic sorted joins work
>> --------------------------------------------
>>   TESTDT[SJ(4, 6), v, mult = "first"] is not identical to 4L. Differences:
>>   Mean relative difference: 0.25
>>   1
>>   Error: Test failures
>>   In addition: Warning message:
>>   In getPackageName(where) :
>>     Created a package name, "2010-12-17 22:23:34", when none found
>>   Execution halted
>> ...
>>
>> You can also run the unit tests in your "live" R session like so:
>>
>> R> library(data.table)
>> R> library(testthat)
>> R> test_dir('/path/to/datatable/pkg/inst/tests')
>>
>> or a single file at a time:
>>
>> R> test_file(''/path/to/datatable/pkg/inst/tests/test-something.R")
>>
>> If you'd consider moving the test suite over to test_that, I'll
>> volunteer to do the conversions over the next week (or so (maybe after
>> xmas)). We can organize the tests a bit more cleanly into different
>> files and "contexts" to be a bit more explicit about which groups of
>> tests are testing what types of functionality if you like.
>>
>> If you don't want to keep track of unit tests by their number, than
>> the conversions will be a bit less cumbersome, but it's your choice.
>>
>> Regardless of the unit test framework switch, my "S4-ism" code is
>> ready to land back in trunk as it stands now. The tests for adding
>> generic methods to an S4 object that uses the data.table won't work
>> with the current test.data.table.R stuff, so I'd just keep those out
>> for now. See the commented part of the testing blocks at the bottom
>> here to see what I mean (tests 228 to 231):
>>
>> https://github.com/lianos/datatable/blob/master/pkg/R/test.data.table.R
>>
>> Sorry for the long email, but there are two issues rolled into one here.
>>
>> Thanks,
>> -steve
>>
>> On Wed, Dec 15, 2010 at 6:47 PM, Matthew Dowle <mdowle at mdowle.plus.com> wrote:
>> > Hi Tom,
>> >
>> > I tested your change to cedta - passes build, tests and vignettes. Looks
>> > good. It didn't fix bug 1131 (I had hoped it might too) but I fixed that
>> > by adding a check on whether the caller is the debugger frame.
>> >
>> > Could you commit your change please then I'll do mine on top.
>> >
>> > Matthew
>> >
>> >
>> > On Mon, 2010-12-06 at 22:37 -0500, Steve Lianoglou wrote:
>> >> Hi,
>> >>
>> >> On Mon, Dec 6, 2010 at 9:17 PM, Matthew Dowle <mdowle at mdowle.plus.com> wrote:
>> >> > Been offline for a few days, just catching up with these threads.
>> >> >
>> >> > Yes - Tom's idea looks good to me. .GlobalEnv would be considered
>> >> > data.table aware since it isn't a namespace then, thus short-circuiting
>> >> > the first || when called from the user workspace and retaining the
>> >> > efficiency fix of the change from cendta to cedta a while back (that the
>> >> > identical(te,.GlobalEnv) was there for) so I'm happy.
>> >> >
>> >> > The topenv when the caller is a namespaceless package is .GlobalEnv
>> >> > rather than the package (if I remember the issue correctly) which is the
>> >> > reason namespaceless packages are incorrectly considered data.table
>> >> > aware. As Tom says that's an issue anyway and shouldn't be affected by
>> >> > this change. In practice it's not an issue since we're not aware of any
>> >> > namespaceless packages being used by data.table users. Have changed the
>> >> > priority of that tracker item to low (which oddly on R-Forge is 1 not
>> >> > 5).
>> >> >
>> >> > The new cedta might well fix this bug neatly too :
>> >> > https://r-forge.r-project.org/tracker/index.php?func=detail&aid=1131&group_id=240&atid=975
>> >> >
>> >> > Haven't looked at the test_that proposal yet. Shall I look once these
>> >> > issues are resolved and a few tests are working ok?
>> >>
>> >> Yeah, I've been meaning to implement Tom's suggestion in my testthat
>> >> branch and send you guys a link to checkout, but I've been busy trying
>> >> to catch up w/ stuff at school.
>> >>
>> >> I'll work on something for you to look at and send you an update in
>> >> the next day or two.
>> >>
>> >> -steve
>> >>
>> >
>> >
>> >
>>
>>
>>
>
>
> _______________________________________________
> datatable-help mailing list
> datatable-help at lists.r-forge.r-project.org
> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/datatable-help
>