fix test-list.R for Windows systems

[Hugovdberg]

A fix for issue #197, the custom filename generator didn't handle the different file path separator on Windows correctly.

[Hugovdberg]

hmm, how do I prevent all those old commits from showing up? Apparently github doesn't notice all those old commits were squashed into the latest merge.. I literally is just one line changed.

[connectedblue]

@Hugovdberg the way I do it is to keep my master an exact copy of this one. Then you create a new branch from master for each feature/bug you want to fix. You then raise a PR from that branch.

[connectedblue]

Hi @Hugovdberg

This fix didn't work unfortunately. I installed off your branch:

devtools::install_github("Hugovdberg/ProjectTemplate@197_fix_test-list")

and I got the same test failures as reported in #197

[Hugovdberg]

I cannot reproduce the errors with the following code:

devtools::install_github("Hugovdberg/ProjectTemplate@197_fix_test-list")
testthat::test_package("ProjectTemplate")

How do you reproduce the errors? I tested it both on windows 7 and windows 10, I only get an error related to missing Perl for the xls.reader. Also, we should fix the test for migration to suppress the warning related to changes in the csv2.reader.

[connectedblue]

Hi @Hugovdberg

I tried your command and got the same 15 failures.

I was running the tests using

source('C:/Users/cs/git/ProjectTemplate2/tests/run-all.R')

and got the same errors.

Here's a snippet:

x[3]: "test/file2ef8665d74d4.csv"
y[3]: "test\\file2ef8665d74d4.csv"

I think there's possibly a difference in how we define the file separator in the environment perhaps? Mine always comes up as \\ for reasons I dont fully understand `

[Hugovdberg]

That's the reason I replace \\ with /. All comparisons are done with the output of list.data first, so the reported y-value is what's created by temp_csv_file inside test-list.R. Apparently not all occurrences of \\ are replaced from the output of tempfile (which confusingly reports a different file separator than list.files). Could you please source the temp_csv_file function and run it to see if any \\ remain?

[connectedblue]

I ran the following:

> temp_csv_file <- function(dir = '') {
+   gsub('^/', '', tempfile(pattern = "file", tmpdir = dir, fileext = ".csv"))
+ }
> temp_csv_file()
[1] "\\file2ef820131766.csv"
> temp_csv_file(dir = 'test')
[1] "test\\file2ef839073b0e.csv"
> 

what's the result supposed to be? should it just be file2ef820131766.csv and test/file2ef839073b0e.csv?

I don't really understand why a tempfile name is being created if the data file is being created inside a project. Can't the test dataframe just be written to files data/test1.csv and data/test2.csv?

[Hugovdberg]

Given the definition of temp_csv_file you show there you did not check out my the branch. See test-list.R in the referenced branch. Could you please make sure you checked out the correct branch or copy the definition from github manually for the test first?

[connectedblue]

ok, I did install your branch directly off github but didn't check it out, so sourcing the files wouldn't have worked.

The tests run fine now.

Is there a particular reason why you selected this strategy for naming a test file? It might be worth placing a comment inside the temp_csv_file function to explain what the nested gsub's and backslashes are doing. Could help future ProjectTemplate maintainers.

[Hugovdberg]

I'm actually not quite sure why I made that decision, because the most feasible argument I can come up with (not hardcoding the correctness of the test) isn't too strong. I did add the explanatory comments as requested.

[KentonWhite]

Cleaning up PRs now that new version is on CRAN.