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

bugfix: auto_test_package does not work on windows machines #465

Merged
merged 3 commits into from
Dec 15, 2016

Conversation

FrancoisGuillem
Copy link
Contributor

On windows, auto_test_package does not automatically run tests when a file is changed. This is because on windows, paths may contain "/" or "".

The problem is solved by calling normalizePath.

@hadley
Copy link
Member

hadley commented Apr 25, 2016

But where are the bad slashes coming from?

@FrancoisGuillem
Copy link
Contributor Author

file.path puts "/" but normalizePath uses only "". Since the watcher calls normalizePath, it expects to see only paths with "".

@kevinushey
Copy link
Collaborator

For reference:

kevinushey@Kevin-MBP-2:~/git/testthat [master]
$ ag -Q "normalizePath"
R/auto-test.R
36:  code_path <- normalizePath(code_path)
37:  test_path <- normalizePath(test_path)
45:    changed <- normalizePath(c(added, modified))
96:    changed <- normalizePath(c(added, modified))

R/source.R
34:  files <- normalizePath(sort(dir(path, pattern, full.names = TRUE)))

R/test-path.R
33:  path <- normalizePath(path)

tests/testthat/test-source_dir.R
7:  res <- source_dir(normalizePath('test_dir'), pattern = 'hello', chdir = TRUE)
13:  res <- source_dir(normalizePath('test_dir'), pattern = 'hello', chdir = FALSE)

Should these all have winslash = "/"?

@FrancoisGuillem
Copy link
Contributor Author

It could help for future developments, but I think the important thing is to always use normalizePath when manipulating paths.

@hadley
Copy link
Member

hadley commented Jul 3, 2016

Could you please add a bullet point to NEWS?

@krlmlr krlmlr added this to the 1.0.3 milestone Jul 4, 2016
@FrancoisGuillem
Copy link
Contributor Author

Done ! Sorry for the delay.

@hadley hadley merged commit 2007a89 into r-lib:master Dec 15, 2016
@hadley
Copy link
Member

hadley commented Dec 15, 2016

Thanks!

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

4 participants