Skip to content

improve test 1864 by using latin1 encoded strings#2715

Merged
mattdowle merged 5 commits intomasterfrom
improve-test-1864
Apr 7, 2018
Merged

improve test 1864 by using latin1 encoded strings#2715
mattdowle merged 5 commits intomasterfrom
improve-test-1864

Conversation

@shrektan
Copy link
Copy Markdown
Member

@shrektan shrektan commented Mar 31, 2018

So it can be tested on all the platforms. It improves the test for PR #2566.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 31, 2018

Codecov Report

Merging #2715 into master will increase coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2715      +/-   ##
==========================================
+ Coverage    93.4%   93.59%   +0.18%     
==========================================
  Files          61       61              
  Lines       12236    12498     +262     
==========================================
+ Hits        11429    11697     +268     
+ Misses        807      801       -6
Impacted Files Coverage Δ
src/openmp-utils.c 89.28% <0%> (+0.39%) ⬆️
R/data.table.R 98.29% <0%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ab4baa...7523b24. Read the comment docs.

Copy link
Copy Markdown
Member

@mattdowle mattdowle left a comment

Choose a reason for hiding this comment

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

test(1864.3, DT1[J(utf8_strings)], DT1[J(latin1_strings)])

Comment thread inst/tests/tests.Rraw Outdated
setkey(DT2, y)
test(1864.1, DT1$x, sort(c(utf8_strings, utf8_strings), method = "radix"))
test(1864.2, DT2$y, sort(c(utf8_strings, utf8_strings), method = "radix"))
test(1864.3, DT1[J(utf8_strings)], DT1[J(latin1_strings)])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Imagine that, for reason of a strange bug, both DT1[J(utf8_strings)] and DT1[J(latin1_strings)] returned NULL. The test would still pass. I like this pattern for that reason :
test(1864.3, DT1[J(utf8_strings)], ans<- data.table( .. known answer .. ) )
test(1864.4, DT1[J(latin1_strings)], ans)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps use the integers in z to construct ans.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Otherwise, great!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You're right. Using an explicit answer is indeed better.

@mattdowle mattdowle added this to the v1.10.6 milestone Apr 7, 2018
@mattdowle mattdowle merged commit af69742 into master Apr 7, 2018
@mattdowle mattdowle deleted the improve-test-1864 branch April 7, 2018 02:33
@shrektan shrektan added the encoding issues related to Encoding label Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

encoding issues related to Encoding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants