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

Use more portable paths in the test explorer #2194

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Feb 2, 2024

Addresses #1808 (comment)

With the R extension's log level set to "debug", you can get this info about the error:

Error: ℹ Loading vscodereporter
Error: '\U' used without hex digits in character string (<input>:1:32)
Execution halted

I was able to reproduce the problem in a windows VM and this fixes it for me.

Relevant context:

  • Prior to this PR, the test-running command looked something like this:

    "C:\Program Files\R\R-4.3.2\bin\R.exe" --no-echo -e "devtools::load_all('d:/Users/jenny/source/repos/positron/extensions/positron-r/resources/testing/vscodereporter');devtools::test_active_file('d:\Users\jenny\source\repos\rematch2\tests\testthat\test-all.R', desc = 'corner cases', reporter = VSCodeReporter)"
    

    Note the paths in the snippet of R code and, specifically, the different path separators. The problem was the Windows-style path where the backslashes aren't escaped, i.e. doubled. In the error above, you can tell that the load_all() command actually succeeds, so it's the path in test_active_file() that is the problem. The solution is to use POSIX-style path separation. Here's how the test-running command looks with this PR:

    "C:\Program Files\R\R-4.3.2\bin\R.exe" --no-echo -e "devtools::load_all('d:/Users/jenny/source/repos/positron/extensions/positron-r/resources/testing/vscodereporter');devtools::test_active_file('d:/Users/jenny/source/repos/rematch2/tests/testthat/test-exec.R',reporter = VSCodeReporter)"
    
  • This treatment was already in place for the path to the embedded test reporter package and now it's clear why.

    const testReporterPath = path
    .join(EXTENSION_ROOT_DIR, 'resources', 'testing', 'vscodereporter')
    .replace(/\\/g, '/');

  • This approach is consistent with the advice given in the R for Windows FAQ:

    Backslashes have to be doubled in R character strings, so for example one needs "d:\\R-4.3.2\\library\\xgobi\\scripts\\xgobi.bat". You can make life easier for yourself by using forward slashes as path separators: they do work under Windows.

  • FWIW child_process.spawn(..., { shell: true }) is launching a cmd.exe shell on Windows, but this turns out to be neither here nor there in terms of this problem.

@jennybc jennybc requested a review from DavisVaughan February 2, 2024 05:08
@jennybc jennybc marked this pull request as ready for review February 2, 2024 05:10
@jennybc jennybc merged commit 289f102 into main Feb 2, 2024
1 check passed
@jennybc jennybc deleted the bugfix/test-feature-windows-paths branch February 2, 2024 16:54
@jennybc jennybc mentioned this pull request Feb 2, 2024
12 tasks
juliasilge added a commit that referenced this pull request Jul 1, 2024
Addresses #3690 in the same manner as we did #2194

### QA Notes

This needs to be checked/tested on Windows.

Take some malformed R code like so:

```r
fn <- function() {
  x <- 1
y <- 2
  x + y
}
```

And then use "Format Document" or (if selected) "Format Selection" from
the command palette.
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.

1 participant