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 novica commented Aug 23, 2019

Tests for clean.R

Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

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

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
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
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
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.

@yihui yihui added this to the v0.16 milestone Oct 1, 2019
Copy link
Member

@yihui yihui left a comment

Choose a reason for hiding this comment

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

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
yihui added a commit that referenced this pull request Oct 1, 2019
@novica
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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants