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 more tests #5

Open
yihui opened this issue Jul 27, 2018 · 18 comments
Open

Add more tests #5

yihui opened this issue Jul 27, 2018 · 18 comments

Comments

@yihui
Copy link
Owner

yihui commented Jul 27, 2018

Coverage is 0% at the moment 😞

https://codecov.io/github/yihui/xfun?branch=master

@shrektan
Copy link
Contributor

I can help for this package. 😃

@yihui
Copy link
Owner Author

yihui commented Jul 28, 2018

Wise choice. I think the task is the easiest in this package compared to the other two. 😁 Thanks!

@shrektan
Copy link
Contributor

Some functions are difficult to test... Finally, I choose to finish all the simpler ones first.

@shrektan
Copy link
Contributor

Still only 21%... 😢 Thought it may go to 50%. I'll come back... 🏃

@yihui
Copy link
Owner Author

yihui commented Nov 27, 2018

It is not necessary to test all functions. I'm totally fine if only simpler tests are done. The detailed statistics here can help you target the functions and lines to test (click on each R script to know which lines are not covered): https://codecov.io/gh/yihui/xfun/tree/master/R

Thanks!

@kishvanchee
Copy link
Contributor

Hi @yihui

I was hoping I can start with something simple. I checked https://codecov.io/gh/yihui/xfun/src/master/R/os.R and gave it a try.

library(testit)
library(xfun)

assert('show proper os', {
  (is_windows() %==% TRUE)
  (is_linux() %==% FALSE)
  (is_macos() %==% FALSE)
  (is_unix() %==% FALSE)
})

All the above tests gave proper results in my windows machine.

> sessionInfo()
R version 3.5.0 (2018-04-23)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252   
[3] LC_MONETARY=English_United States.1252 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] xfun_0.8   testit_0.9

loaded via a namespace (and not attached):
[1] compiler_3.5.0 tools_3.5.0    rstudioapi_0.7 yaml_2.1.19 

I can try testing this again on my linux system at home. Other than that, I don't have a mac so I'm not sure how to test on a mac system.

Please let me know how to proceed further. I'm not sure how to make the codecove link to become a 100% (?).

If this works out then I can try pitching in for other scripts. :)

@yihui
Copy link
Owner Author

yihui commented Aug 19, 2019

@kishvanchee Testing these OS-related functions on different OSes is a little tricky and probably unnecessary, since they are super simple functions. I'd suggest you work on other functions instead. Thank you!

@kishvanchee
Copy link
Contributor

kishvanchee commented Aug 20, 2019

@yihui Understood. I thought of starting with markdown.R now. I am trying to understand why the tests failed. Can you please share examples for the same? I can see from the doc strings it's performing the required function and gives the appropriate output.

@yihui
Copy link
Owner Author

yihui commented Aug 20, 2019

Sorry I don't understand it. What do you mean by "examples for the same"? Where are your tests? You could submit a PR even if you haven't finished it, so I can see how the tests are failing on Travis CI.

@kishvanchee
Copy link
Contributor

Apologies for not being clear. Looking at the red highlight I understood that those are the lines that fail. So that is why I was asking for the examples - I meant the tests for which they failed to make them highlight as red.

Ok, to clarify, how exactly should I go about this? Currently, I installed the package(s) and did assert (conditions). Should I be cloning this repo and run some script to test things out?

@yihui
Copy link
Owner Author

yihui commented Aug 20, 2019

I see. One simple example:

assert('protect_math() puts inline math expressions in backticks', {
  (xfun::protect_math('$x$') %==% '`\\(x\\)`')
})

You don't need to test anything locally. You can just submit a PR and let Travis perform R CMD check. If you are not familiar with Github pull requests, you may have to read the Github help pages first.

@kishvanchee
Copy link
Contributor

Ok, so I poked around a little, I just noticed the tests/testit/ folder. I had a look at test-markdown.R. I notice that the tests exist only for prose_index(). If I understand correctly, this is why I see all green for the prose_index() in the codecov link, and red for others. It's not that they fail at tests, it's that there is no test at all in the first place. Same can be said about protect_math() function as you have posted above.

Am I correct in understanding it now?

Ok cool. I can perform a PR. I am familiar with it. Hope I can help with your package in some way. :)

@yihui
Copy link
Owner Author

yihui commented Aug 20, 2019

Yes, you are 100% correct! (Red lines means they are not covered by any tests)

@bblodfon
Copy link
Contributor

bblodfon commented Sep 7, 2020

Hi! I wanted to add some more tests and I stumbled upon some things that I need to clear up first:

  1. When I ran test_pkg('xfun') from the RStudio console, I get an output of error strings and 1 warning:
Error in base64_decode("lz..??") : 
  The input string is not a valid base64 encoded string
Error in read_utf8(mixed_file2, error = TRUE) : 
  The file /tmp/RtmpFg3Elp/file6abd57ddcd29 is not encoded in UTF-8. These lines contain invalid UTF-8 characters: 4
Error in tojson(Sys.Date()) : The class of x is not supported: Date
Error in tojson(NA) : 
  Logical values of length > 1 and NA are not supported
Error in loadable(c("base", "base2")) : 
  'pkg' must be a character vector of length one
In addition: Warning message:
In prose_index(x) : Code fences are not balanced
Error in loadable(character()) : 
  'pkg' must be a character vector of length one
Error in with_ext("abc", NA_character_) : NA is not allowed in 'ext'
Error in with_ext(c("a", "b"), c("d", "e", "f")) : 
  'ext' must be of the same length as 'x'
Error in n2w(1e+15) : The absolute value must be less than 1e15.

I guess that is the expected output, right? (we just see the error messages here, not that the tests are wrong). Do we want to supress these kind of messages? Especially the 1 warning since it's been outputed in the (2) below.

  1. When I run the tests directly from RStudio (Ctrl+Shift+T), I get (apart from the warning) this error:

Error in base64_decode(output) : could not find function "base64_decode", which comes from this test:

assert('base64_decode() decodes the string correctly', {
  (sapply(c(1:10, 255:246), function(i) {
    input = as.raw(1:i)
    output = base64_encode(input)
    input2 = base64_decode(output)
    input %==% input2
  }))
})

and this one: Error in R_logo() : could not find function "R_logo", having to do with the test:

assert('base64_encode_r() returns the same result as base64_encode()', {
  f = R_logo()
  base64_encode_r(f) %==% base64_encode(f)
})

So, is it just me that gets this errors when running via the (2) way the tests? Shouldn't we have the same output as in the first case or should I just not use the (2) way? Notably, the second does not work in that case even if I put xfun:::R_logo()!

@yihui
Copy link
Owner Author

yihui commented Sep 8, 2020

@bblodfon For 2, you need to install the package (Ctrl + Shift + B) before Ctrl + Shift + T. For 1, I always do Ctrl + Shift + T in RStudio, and I've never noticed these problems, but I think you can ignore them.

@bblodfon
Copy link
Contributor

bblodfon commented Sep 8, 2020

I have started writing some tests in a separate branch - @yihui would you mind giving me access to push it upstream at some point? (I will send a PR when I am done)

@yihui
Copy link
Owner Author

yihui commented Sep 8, 2020

@bblodfon You don't need the write access to this repo. You can do everything in your own fork, and send a PR. Thanks!

yihui added a commit to yihui/testit that referenced this issue Sep 22, 2020
…sts; the temp directory will be deleted after the R session ends

this will no longer be necessary: yihui/xfun#5 (comment)
@yihui
Copy link
Owner Author

yihui commented Oct 1, 2020

@bblodfon FYI, the Ctrl + Shift + B step I mentioned above is unnecessary now with the latest version of testit (on CRAN). Ctrl + Shift + T will do the installation first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants