[datatable-help] Streamlining unit tests with test_that

[Matthew Dowle]

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
>