-
Notifications
You must be signed in to change notification settings - Fork 159
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
fix test-list.R for Windows systems #199
fix test-list.R for Windows systems #199
Conversation
…dated documentation
… with clear(), testing for cache ignored Added data_ignore to project.config
… into add_list.data
…; use .read.config in load tests
…_loading; Added more comments
… into add_list.data
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. |
@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. |
Hi @Hugovdberg This fix didn't work unfortunately. I installed off your branch:
and I got the same test failures as reported in #197 |
I cannot reproduce the errors with the following code:
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 |
Hi @Hugovdberg I tried your command and got the same 15 failures. I was running the tests using
and got the same errors. Here's a snippet:
I think there's possibly a difference in how we define the file separator in the environment perhaps? Mine always comes up as |
That's the reason I replace |
I ran the following:
what's the result supposed to be? should it just be 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 |
Given the definition of |
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 |
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. |
1e74500
to
acf658b
Compare
Cleaning up PRs now that new version is on CRAN. |
A fix for issue #197, the custom filename generator didn't handle the different file path separator on Windows correctly.