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.

Sign up for GitHub

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

Jump to bottom

Add errors on separate lines for coverage #4301

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft

Add errors on separate lines for coverage #4301

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1507209
add whitespace to remove spurious coverage of inlined src/ error bran…
Mar 13, 2020
602b194
include regex to CRAN_Release
Mar 13, 2020
b8db05d
do warnings as well
Mar 13, 2020
d1f9dc5
done STOP
Mar 13, 2020
806a580
nothing for DTWARN
Mar 13, 2020
77e8762
adding some coverage tests for newly uncovered lines
Mar 14, 2020
1d5c7ac
typo
Mar 14, 2020
876f693
small fixes
Mar 14, 2020
1b0122e
more tests
Mar 14, 2020
914b749
fix some tests, typos
Mar 14, 2020
b3502bc
more tests & nocov
Mar 14, 2020
c055e43
more tests
Mar 15, 2020
e1b1550
typos
Mar 15, 2020
e8c41ea
more tests, nocov
Mar 15, 2020
9183810
not really reproducing the CI errors locally. managed to get two on -…
Mar 15, 2020
c524c56
found the typo
Mar 15, 2020
75ba288
two more files
Mar 15, 2020
baeb768
+tests
Mar 15, 2020
a3c4fa6
test #s
Mar 15, 2020
5611fee
one more spate of tests for the night
Mar 15, 2020
3c87dd7
cover all but one line
Mar 16, 2020
95e88f8
missed nocov
Mar 16, 2020
4e69db6
one more nocov
Mar 16, 2020
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
8 changes: 8 additions & 0 deletions .dev/CRAN_Release.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ grep -nE "(llu|lld|zd|zu)" src/*.[hc]
grep -P "\t" ./R/*.R
grep -P "\t" ./src/*.c

# all error branches should be on a new line to be properly included in codecov; exceptions:
# (1) in a comment (pattern: after // or after \s*[*]
# (2) in '} else error(' pattern (if else is covered, so is the error)
# (3) in #define macros
for type in error warning STOP DTWARN Error; do
grep -Enr "\b$type[(]" src --include=*.c | grep -Ev "^src\/[a-zA-Z-]+[.]c:[0-9]+:\s*(?:(?:[}]?\s*else\s*)?$type|#define|[*]|.*\/\/.*$type)"
done

# No T or F symbols in tests.Rraw. 24 valid F (quoted, column name or in data) and 1 valid T at the time of writing
grep -n "[^A-Za-z0-9]T[^A-Za-z0-9]" ./inst/tests/tests.Rraw
grep -n "[^A-Za-z0-9]F[^A-Za-z0-9]" ./inst/tests/tests.Rraw
Expand Down
34 changes: 31 additions & 3 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6312,15 +6312,13 @@ options(datatable.optimize = Inf)

# fread dec=',' e.g. France
test(1439, fread("A;B\n1;2,34\n", dec="12"), error="nchar(dec) == 1L is not TRUE")
test(1440, fread("A;B\n8;2,34\n", dec="1"), data.table(A=8L, B="2,34"))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!dec %chin% c('.', ',') is implied incorrect by L1179 of fread.c @ master:

dec='' not allowed. Should be '.' or ','

?fread is not as strict:

The decimal separator as in utils::read.csv. If not "." (default) then usually ",". See details.

Took a conservative approach for now, and blocked !dec %chin% c(',', '.'). But that broke two tests (1440 and 1444.2 here).

dec='1' and dec='*' ran without errors... happy to restore this to work, and change the error/message found in fread.c.

test(1441, fread("A;B\n8;2,34\n", dec=","), data.table(A=8L, B=2.34))
test(1442, fread("A;B\n1;2,34\n", sep=".", dec="."), error="sep == dec ('.') is not allowed")
test(1443, fread("A;B\n1;2,34\n", dec=",", sep=","), error="sep == dec (',') is not allowed")

# sep=".", issue #502
input = paste( paste("192.168.4.", 1:10, sep=""), collapse="\n")
test(1444.1, fread(input, sep=".", dec="*"), ans<-data.table(V1=192L,V2=168L,V3=4L,V4=1:10))
test(1444.2, fread(input, sep=".", dec=","), ans)
test(1444.2, fread(input, sep=".", dec=","), ans<-data.table(V1=192L,V2=168L,V3=4L,V4=1:10))
test(1444.3, fread(paste(paste("192. 168. 4. ", 1:10, sep = ""), collapse="\n"), sep=".", dec=","), ans)
test(1444.4, fread(paste(paste("Hz.BB.GHG.", 1:10, sep = ""), collapse="\n"), sep=".", dec=","),
data.table(V1="Hz",V2="BB",V3="GHG",V4=1:10))
Expand Down Expand Up @@ -16846,3 +16844,33 @@ A = data.table(A=c(complex(real = 1:3, imaginary=c(0, -1, 1)), NaN))
test(2138.3, rbind(A,B), data.table(A=c(as.character(A$A), B$A)))
A = data.table(A=as.complex(rep(NA, 5)))
test(2138.4, rbind(A,B), data.table(A=c(as.character(A$A), B$A)))

# tests caught by introducing newlines in #4301
test(2139.01, getDTthreads(8L), error="'verbose' must be TRUE or FALSE")
test(2139.02, setDTthreads(1:2), error='threads= must be either NULL.*length 2')
test(2139.03, setDTthreads(1+0i), error='threads= must be either NULL.*type integer/numeric')
DT = data.table(1L)
invisible(alloc.col(DT, 10010L))
test(2139.04, alloc.col(DT), DT, warning='greater than 10,000 items over-allocated')
test(2139.05, alloc.col(DT, verbose = 1L), error="verbose must be TRUE or FALSE")
test(2139.06, set(data.table(NULL), NULL, 1L, 1L), error="Input data.table has no columns")
test(2139.07, set(1L, NULL, 1L, 1L), error="object passed to assign isn't type VECSXP")
names(DT) = NULL
test(2139.08, set(DT, NULL, 1L, 2L), error="data.table passed to assign has no names")
test(2139.09, alloc.col(NULL), error="object passed to alloccol is NULL")
test(2139.10, alloc.col(1L), error="object passed to alloccol isn't type VECSXP")
test(2139.11, set(NULL, NULL, 1L, 1L), error="object passed to assign is NULL")
setDF(DT)
names(DT) = 'V1'
test(2139.12, set(DT, NULL, 'V2', 2L), error="set() on a data.frame is for changing existing columns")
setDT(DT)
invisible(alloc.col(DT, 10010L))
test(2139.13, set(DT, NULL, 'V2', 2L), DT, warning="greater than 10,000 items over-allocated")

txt = 'a,b\n1,2'
test(2139.14, fread(txt, sep=',', quote=','), error="sep == quote (',') is not allowed")
test(2139.15, fread(txt, dec='&'), error="dec='&' not allowed")
test(2139.16, fread(txt, quote='.', dec='.'), error="quote == dec ('.') is not allowed")

test(2139.17, setNumericRounding(1:2), error="Rounding level must be a length-1 integer or numeric vector")
test(2139.18, setNumericRounding(-1), error="Rounding level must be 2, 1 or 0")
118 changes: 80 additions & 38 deletions src/assign.c
View file
Open in desktop

Large diffs are not rendered by default.

48 changes: 32 additions & 16 deletions src/bmerge.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,33 +45,44 @@ SEXP bmerge(SEXP iArg, SEXP xArg, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SE

// iArg, xArg, icolsArg and xcolsArg
i = iArg; x = xArg; // set globals so bmerge_r can see them.
if (!isInteger(icolsArg)) error(_("Internal error: icols is not integer vector")); // # nocov
if (!isInteger(xcolsArg)) error(_("Internal error: xcols is not integer vector")); // # nocov
if (LENGTH(icolsArg) > LENGTH(xcolsArg)) error(_("Internal error: length(icols) [%d] > length(xcols) [%d]"), LENGTH(icolsArg), LENGTH(xcolsArg)); // # nocov
if (!isInteger(icolsArg))
error(_("Internal error: icols is not integer vector")); // # nocov
if (!isInteger(xcolsArg))
error(_("Internal error: xcols is not integer vector")); // # nocov
if (LENGTH(icolsArg) > LENGTH(xcolsArg))
error(_("Internal error: length(icols) [%d] > length(xcols) [%d]"), LENGTH(icolsArg), LENGTH(xcolsArg)); // # nocov
icols = INTEGER(icolsArg);
xcols = INTEGER(xcolsArg);
xN = LENGTH(x) ? LENGTH(VECTOR_ELT(x,0)) : 0;
iN = ilen = anslen = LENGTH(i) ? LENGTH(VECTOR_ELT(i,0)) : 0;
ncol = LENGTH(icolsArg); // there may be more sorted columns in x than involved in the join
for(int col=0; col<ncol; col++) {
if (icols[col]==NA_INTEGER) error(_("Internal error. icols[%d] is NA"), col); // # nocov
if (xcols[col]==NA_INTEGER) error(_("Internal error. xcols[%d] is NA"), col); // # nocov
if (icols[col]>LENGTH(i) || icols[col]<1) error(_("icols[%d]=%d outside range [1,length(i)=%d]"), col, icols[col], LENGTH(i));
if (xcols[col]>LENGTH(x) || xcols[col]<1) error(_("xcols[%d]=%d outside range [1,length(x)=%d]"), col, xcols[col], LENGTH(x));
if (icols[col]==NA_INTEGER)
error(_("Internal error. icols[%d] is NA"), col); // # nocov
if (xcols[col]==NA_INTEGER)
error(_("Internal error. xcols[%d] is NA"), col); // # nocov
if (icols[col]>LENGTH(i) || icols[col]<1)
error(_("icols[%d]=%d outside range [1,length(i)=%d]"), col, icols[col], LENGTH(i));
if (xcols[col]>LENGTH(x) || xcols[col]<1)
error(_("xcols[%d]=%d outside range [1,length(x)=%d]"), col, xcols[col], LENGTH(x));
int it = TYPEOF(VECTOR_ELT(i, icols[col]-1));
int xt = TYPEOF(VECTOR_ELT(x, xcols[col]-1));
if (iN && it!=xt) error(_("typeof x.%s (%s) != typeof i.%s (%s)"), CHAR(STRING_ELT(getAttrib(x,R_NamesSymbol),xcols[col]-1)), type2char(xt), CHAR(STRING_ELT(getAttrib(i,R_NamesSymbol),icols[col]-1)), type2char(it));
if (iN && it!=xt)
error(_("typeof x.%s (%s) != typeof i.%s (%s)"), CHAR(STRING_ELT(getAttrib(x,R_NamesSymbol),xcols[col]-1)), type2char(xt), CHAR(STRING_ELT(getAttrib(i,R_NamesSymbol),icols[col]-1)), type2char(it));
}
// raise(SIGINT);

// rollArg, rollendsArg
roll = 0.0; rollToNearest = FALSE;
if (isString(rollarg)) {
if (strcmp(CHAR(STRING_ELT(rollarg,0)),"nearest") != 0) error(_("roll is character but not 'nearest'"));
if (TYPEOF(VECTOR_ELT(i, icols[ncol-1]-1))==STRSXP) error(_("roll='nearest' can't be applied to a character column, yet."));
if (strcmp(CHAR(STRING_ELT(rollarg,0)),"nearest") != 0)
error(_("roll is character but not 'nearest'"));
if (TYPEOF(VECTOR_ELT(i, icols[ncol-1]-1))==STRSXP)
error(_("roll='nearest' can't be applied to a character column, yet."));
roll=1.0; rollToNearest=TRUE; // the 1.0 here is just any non-0.0, so roll!=0.0 can be used later
} else {
if (!isReal(rollarg)) error(_("Internal error: roll is not character or double")); // # nocov
if (!isReal(rollarg))
error(_("Internal error: roll is not character or double")); // # nocov
roll = REAL(rollarg)[0]; // more common case (rolling forwards or backwards) or no roll when 0.0
}
rollabs = fabs(roll);
Expand Down Expand Up @@ -153,7 +164,8 @@ SEXP bmerge(SEXP iArg, SEXP xArg, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SE
// xo arg
xo = NULL;
if (length(xoArg)) {
if (!isInteger(xoArg)) error(_("Internal error: xoArg is not an integer vector")); // # nocov
if (!isInteger(xoArg))
error(_("Internal error: xoArg is not an integer vector")); // # nocov
xo = INTEGER(xoArg);
}

Expand Down Expand Up @@ -268,7 +280,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
case LT : xupp = xlow + 1; xlow = xlowIn; break;
case GE : if (ival.i != NA_INTEGER) xupp = xuppIn; break;
case GT : xlow = xupp - 1; if (ival.i != NA_INTEGER) xupp = xuppIn; break;
default : error(_("Internal error in bmerge_r for '%s' column. Unrecognized value op[col]=%d"), type2char(TYPEOF(xc)), op[col]); // #nocov
default :
error(_("Internal error in bmerge_r for '%s' column. Unrecognized value op[col]=%d"), type2char(TYPEOF(xc)), op[col]); // #nocov
}
// for LE/LT cases, we need to ensure xlow excludes NA indices, != EQ is checked above already
if (op[col] <= 3 && xlow<xupp-1 && ival.i != NA_INTEGER && INTEGER(xc)[XIND(xlow+1)] == NA_INTEGER) {
Expand Down Expand Up @@ -300,7 +313,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
}
break;
case STRSXP : {
if (op[col] != EQ) error(_("Only '==' operator is supported for columns of type %s."), type2char(TYPEOF(xc)));
if (op[col] != EQ)
error(_("Only '==' operator is supported for columns of type %s."), type2char(TYPEOF(xc)));
ival.s = ENC2UTF8(STRING_ELT(ic,ir));
while(xlow < xupp-1) {
int mid = xlow + (xupp-xlow)/2;
Expand Down Expand Up @@ -376,7 +390,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
case LT : xupp = xlow + 1; if (!isivalNA) xlow = xlowIn; break;
case GE : if (!isivalNA) xupp = xuppIn; break;
case GT : xlow = xupp - 1; if (!isivalNA) xupp = xuppIn; break;
default : error(_("Internal error in bmerge_r for '%s' column. Unrecognized value op[col]=%d"), type2char(TYPEOF(xc)), op[col]); // #nocov
default :
error(_("Internal error in bmerge_r for '%s' column. Unrecognized value op[col]=%d"), type2char(TYPEOF(xc)), op[col]); // #nocov
}
// for LE/LT cases, we need to ensure xlow excludes NA indices, != EQ is checked above already
if (op[col] <= 3 && xlow<xupp-1 && !isivalNA && (!isInt64 ? ISNAN(dxc[XIND(xlow+1)]) : (DtoLL(dxc[XIND(xlow+1)]) == NA_INT64_LL))) {
Expand Down Expand Up @@ -465,7 +480,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
}
} else if (roll!=0.0 && col==ncol-1) {
// runs once per i row (not each search test), so not hugely time critical
if (xlow != xupp-1 || xlow<xlowIn || xupp>xuppIn) error(_("Internal error: xlow!=xupp-1 || xlow<xlowIn || xupp>xuppIn")); // # nocov
if (xlow != xupp-1 || xlow<xlowIn || xupp>xuppIn)
error(_("Internal error: xlow!=xupp-1 || xlow<xlowIn || xupp>xuppIn")); // # nocov
if (rollToNearest) { // value of roll ignored currently when nearest
if ( (!lowmax || xlow>xlowIn) && (!uppmax || xupp<xuppIn) ) {
if ( ( TYPEOF(ic)==REALSXP && REAL(ic)[ir]-REAL(xc)[XIND(xlow)] <= REAL(xc)[XIND(xupp)]-REAL(ic)[ir] ) // TODO remove macro here requires bigger rewrite
Expand Down
Loading