Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add errors on separate lines for coverage #4301

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ if (exists("test.data.table", .GlobalEnv, inherits=FALSE)) {
test = data.table:::test
uniqlengths = data.table:::uniqlengths
uniqlist = data.table:::uniqlist
veqseq = data.table:::vecseq
which_ = data.table:::which_
which.first = data.table:::which.first
which.last = data.table:::which.last
Expand Down Expand Up @@ -16909,3 +16910,9 @@ test(2139.42, DT[ , prod(b, na.rm=1), by=a], error="na.rm must be TRUE or FALSE"
test(2139.43, DT[ , prod(l), by=a], error="GForce prod can only be applied to columns, not .SD or similar")
test(2139.44, DT[ , prod(f), by=a], error="prod is not meaningful for factors")
options(old)

test(2139.45, vecseq(NA, NA, NA), error="x must be an integer vector")
test(2139.46, vecseq(1L, NA, NA), error="len must be an integer vector")
test(2139.47, vecseq(1L, 1:2, NA), error="x and len must be the same length")
test(2139.48, vecseq(1L, 1L, 1L), error="clamp must be a double vector length 1")
test(2139.49, vecseq(1L, 1L, -1), error="clamp must be positive")
16 changes: 8 additions & 8 deletions src/dogroups.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
if (!isNull(xjiscols) && LENGTH(order) && !LOGICAL(on)[0])
error(_("Internal error: xjiscols not NULL but o__ has length")); // # nocov
if(!isEnvironment(env))
error(_("'env' should be an environment"));
error(_("Internal error: 'env' should be an environment")); // # nocov
ngrp = length(starts); // the number of groups (nrow(groups) will be larger when by)
ngrpcols = length(grpcols);
nrowgroups = length(VECTOR_ELT(groups,0));
Expand All @@ -48,7 +48,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
setAttrib(BY, R_NamesSymbol, bynames); // Fix for #42 - BY doesn't retain names anymore
R_LockBinding(sym_BY, env);
if (isNull(jiscols) && (length(bynames)!=length(groups) || length(bynames)!=length(grpcols)))
error(_("!length(bynames)[%d]==length(groups)[%d]==length(grpcols)[%d]"),length(bynames),length(groups),length(grpcols));
error(_("Internal error: !length(bynames)[%d]==length(groups)[%d]==length(grpcols)[%d]"), length(bynames), length(groups), length(grpcols)); // # nocov
// TO DO: check this check above.

N = PROTECT(findVar(install(".N"), env)); nprotect++; // PROTECT for rchk
Expand Down Expand Up @@ -77,7 +77,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
// using <- in j (which is valid, useful and tested), they are repointed to the .SD cols for each group.
SEXP names = PROTECT(getAttrib(SDall, R_NamesSymbol)); nprotect++;
if (length(names) != length(SDall))
error(_("length(names)!=length(SD)"));
error(_("Internal erro: length(names)!=length(.SD)")); // # nocov
jangorecki marked this conversation as resolved.
Show resolved Hide resolved
SEXP *nameSyms = (SEXP *)R_alloc(length(names), sizeof(SEXP));
for(int i=0; i<length(SDall); ++i) {
if (SIZEOF(VECTOR_ELT(SDall, i))==0)
Expand All @@ -92,7 +92,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX

SEXP xknames = PROTECT(getAttrib(xSD, R_NamesSymbol)); nprotect++;
if (length(xknames) != length(xSD))
error(_("length(xknames)!=length(xSD)"));
error(_("Internal error: length(xknames)!=length(xSD)")); // # nocov
SEXP *xknameSyms = (SEXP *)R_alloc(length(xknames), sizeof(SEXP));
for(int i=0; i<length(xSD); ++i) {
if (SIZEOF(VECTOR_ELT(xSD, i))==0)
Expand All @@ -101,9 +101,9 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
}

if (length(iSD)!=length(jiscols))
error(_("length(iSD)[%d] != length(jiscols)[%d]"),length(iSD),length(jiscols));
error(_("Internal error: length(iSD)[%d] != length(jiscols)[%d]"), length(iSD), length(jiscols)); // # nocov
if (length(xSD)!=length(xjiscols))
error(_("length(xSD)[%d] != length(xjiscols)[%d]"),length(xSD),length(xjiscols));
error(_("Internal error: length(xSD)[%d] != length(xjiscols)[%d]"), length(xSD), length(xjiscols)); // # nocov

SEXP listwrap = PROTECT(allocVector(VECSXP, 1)); nprotect++;
Rboolean jexpIsSymbolOtherThanSD = (isSymbol(jexp) && strcmp(CHAR(PRINTNAME(jexp)),".SD")!=0); // test 559
Expand Down Expand Up @@ -325,7 +325,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
estn = ((double)ngrp/i)*1.1*(ansloc+maxn);
if (verbose) Rprintf(_("dogroups: growing from %d to %d rows\n"), length(VECTOR_ELT(ans,0)), estn);
if (length(ans) != ngrpcols + njval)
error(_("dogroups: length(ans)[%d]!=ngrpcols[%d]+njval[%d]"),length(ans),ngrpcols,njval);
error(_("Internal error in dogroups: length(ans)[%d]!=ngrpcols[%d]+njval[%d]"), length(ans), ngrpcols, njval); // # nocov
for (int j=0; j<length(ans); ++j) SET_VECTOR_ELT(ans, j, growVector(VECTOR_ELT(ans,j), estn));
}
}
Expand Down Expand Up @@ -407,7 +407,7 @@ SEXP growVector(SEXP x, const R_len_t newlen)
SEXP newx;
R_len_t len = length(x);
if (isNull(x))
error(_("growVector passed NULL"));
error(_("Internal error: growVector passed NULL")); // # nocov
PROTECT(newx = allocVector(TYPEOF(x), newlen)); // TO DO: R_realloc(?) here?
if (newlen < len) len=newlen; // i.e. shrink
switch (TYPEOF(x)) {
Expand Down
2 changes: 1 addition & 1 deletion src/vecseq.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ SEXP vecseq(SEXP x, SEXP len, SEXP clamp)
reslen += ilen[i];
}
if (!isNull(clamp)) {
if (!isNumeric(clamp) || LENGTH(clamp)!=1)
if (TYPEOF(clamp) != REALSXP || LENGTH(clamp)!=1)
error(_("clamp must be a double vector length 1"));
double limit = REAL(clamp)[0];
if (limit<0)
Expand Down
6 changes: 3 additions & 3 deletions src/wrappers.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ SEXP setlistelt(SEXP l, SEXP i, SEXP value)
R_len_t i2;
// Internal use only. So that := can update elements of a list of data.table, #2204. Just needed to overallocate/grow the VECSXP.
if (!isNewList(l))
error(_("First argument to setlistelt must be a list()"));
error(_("Internal error: first argument to setlistelt must be a list()")); // # nocov
if (!isInteger(i) || LENGTH(i)!=1)
error(_("Second argument to setlistelt must a length 1 integer vector"));
error(_("Internal error: second argument to setlistelt must a length 1 integer vector")); // # nocov
i2 = INTEGER(i)[0];
if (LENGTH(l) < i2 || i2<1)
error(_("i (%d) is outside the range of items [1,%d]"),i2,LENGTH(l));
error(_("Internal error: i (%d) is outside the range of items [1,%d]"), i2, LENGTH(l)); // # nocov
SET_VECTOR_ELT(l, i2-1, value);
return(R_NilValue);
}
Expand Down