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

new file: tests/testit/test-clean.R #405

Merged
merged 6 commits into from Oct 1, 2019
Merged

new file: tests/testit/test-clean.R #405

merged 6 commits into from Oct 1, 2019

Conversation

@novica
Copy link
Contributor

novica commented Aug 23, 2019

Tests for clean.R

Copy link
Member

yihui left a comment

It is not a good idea to copy the source code from R/ to tests/. When the source code under R/ is updated, you'd have to copy again. Are you able to come up with a way to avoid copying? Thanks!

@novica

This comment has been minimized.

Copy link
Contributor Author

novica commented Aug 23, 2019

I thought this was not the way to go but couldn't figure so far how to actually call the functions that do the string operations because as far as I understand all are anonymous functions.

Maybe another approach would be to modify clean.R but yeah not sure if this is what is needed. For example:

# replace three or more \n with two, i.e. two or more empty lines with one
replace_inline = function(x) {
  x = paste(gsub('\\s+$', '', x), collapse = '\n')
  trim_ws(gsub('\n{3,}', '\n\n', x))
}
remove_extra_empty_lines = function(f) process_file(f, remove_inline)
@novica

This comment has been minimized.

Copy link
Contributor Author

novica commented Aug 24, 2019

So, now the functions in clean.R are no longer anonymous and the tests call them. However, it seems like I am missing something because I always get an error in the tests about not finding the fucntion. I thought that this edits would work when comparing to what I did for the test-addin.R.

@novica

This comment has been minimized.

Copy link
Contributor Author

novica commented Sep 7, 2019

It took me some time to figure this out -- I don't know if it my setup or something else but I had to add devtools::load_all('..') to tests/test-all.R in order to get testit to check the changed clean.R script.

It seems to me that otherwise testit was testing against the installed blogdown package and not the repository I have cloned. ctr+shift+L for devtools::load_all(".") doesn't seem to be enough for the tests to run.

novica and others added 4 commits Aug 23, 2019
	modified:   tests/test-all.R
	modified:   tests/testit/test-clean.R
…sible to return the processed character vector, so that we can test these functions without reading/writing to files
@yihui yihui force-pushed the novica:tests branch from ad5c609 to 39fb764 Oct 1, 2019
@yihui yihui added this to the v0.16 milestone Oct 1, 2019
@yihui
yihui approved these changes Oct 1, 2019
Copy link
Member

yihui left a comment

I added a third argument x to process_file() so we can test the functions without reading or writing to files. Thank you!

BTW, if you use the dev version of testit (remotes::install_github('yihui/testit')), you will be able to Ctrl + Shift + L and then run test-all.R.

@yihui yihui merged commit 9da9136 into rstudio:master Oct 1, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
yihui added a commit that referenced this pull request Oct 1, 2019
@novica

This comment has been minimized.

Copy link
Contributor Author

novica commented Oct 2, 2019

Hi Yihui. Thanks for looking into this. I was thinking that I am not doing this right because there was no feedback for a while. I will try to work on the rest of the R files now for tests.

@yihui

This comment has been minimized.

Copy link
Member

yihui commented Oct 2, 2019

No, it was just that I had been busy with many other things. Sorry about the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.