style_files -> vectorized style_file #273
Conversation
|
I prefer the latter option, keep API and rename |
| @@ -10,12 +10,17 @@ test_that("styler can style directory", { | |||
| expect_false(all(style_dir(testthat_file("public-api", "xyzdir")))) | |||
| }) | |||
|
|
|||
| test_that("styler can style file", { | |||
| test_that("styler can style files", { | |||
There was a problem hiding this comment.
Do we need to test character(0) as argument, or a TRUE return value if contents change?
There was a problem hiding this comment.
Not sure if I understand you correctly. Well, the idea was just to check whether calling the function causes an error or not. Since the files were already styled, we don't expect any changes. So you wanted to test whether the output of style_file() is character(0)?
There was a problem hiding this comment.
Seems that style_file() returns a logical vector, but the test only tests FALSE return values. Is there a test for a TRUE return value? If not, we might need one.
Likewise, empty input should lead to logical(0) .
There was a problem hiding this comment.
Yes we have never explicitly tested for a return value other than TRUE because the majority of our tests works with test_collection() that transforms an input with a transformer function (in our case style_text()) and compares the output to a reference.
Do you think we need a test for it? If yes, we'd probably best write a function that takes as input the path of a mal-styled file, copies it to a temp directory, styles it, saves the direct return value of style_file()and then discards the files in the temp directory. I just hoped to get around that because it may bear additional problems with paths for covr or testthat for limited gain, but if you want, I can implement that.
There was a problem hiding this comment.
No, that's not a priority, I was just wondering when looking at the tests.
lorenzwalthert
force-pushed
the
style-files
branch
from
41e48a0 to
891f9ef
Compare
lorenzwalthert
force-pushed
the
style-files
branch
from
891f9ef to
d760b7c
Compare
Codecov Report
@@ Coverage Diff @@
## master #273 +/- ##
==========================================
+ Coverage 92.87% 93.16% +0.28%
==========================================
Files 28 28
Lines 1123 1155 +32
==========================================
+ Hits 1043 1076 +33
+ Misses 80 79 -1
Continue to review full report at Codecov.
|
- Adapt documentation (#290). - Add roundtrip (#287). - Fix AppVeyor builds. - Fix token insertion / comment interaction (#279). - Clarify labelling strategy (#285). - Fixing and extending Rstudioaddins (#283). - Fix eq assign parsing (#276). - style_files -> vectorized style_file (#273). - Refactoring (#270). - Fix CI (#275). - Fix covr (#274). - Renaming files (#271). - Handle styling of an unsaved active file (#243). - Test R 3.1 and R 3.2 (#249). - Allow empty {} without line break (#261). - Wrap expr in expr before enclosing with curly braces (#263). - Avoid checking for hard-coded dot (#262). - Account for dependency renaming (utf8 changed to enc) (#264). - Indention of function declaration and closing braces (#260). - Only remove line break before closing with strict option (#252).
Closes #219. Should we depreciate
style_file()in favour ofstyle_files()or should we renamestyle_files()tostyle_file()and only use the current implementation ofstyle_file()internally (e.g. asstyle_file_one())?