-
Notifications
You must be signed in to change notification settings - Fork 301
Clean up code and tests around nested projects #1085
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
Conversation
Trying to make it more clear when we use it for a reason related to nested projects
| } | ||
|
|
||
| # creates a backdoor we can exploit in tests | ||
| allow_nested_project <- function() FALSE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new way we give ourselves of method of creating a nested project in tests. More self-explaining than previous reliance on is_testing() plus a magic string in the project name.
|
|
||
| ## putting `pattern` in the package or project name is part of our strategy for | ||
| ## suspending the nested project check during testing | ||
| pattern <- "aaa" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I'm not sure why I ever made the magic string part of the default project name.
Probably because I was very frustrated with problems around nested projects and testing? If this ever happens to you @hadley (catastrophic test failure related to nested projects), remember we have some code elsewhere in this helper.R file that can be run to figure out why session temp dir suddenly looks like a project (usually because it somehow holds one or more .Rproj files). Then we can delete the offending file(s). This is something RStudio does in the background every now and then and I've never been able to figure out why.
| child <- create_project(path(parent, "fghijk")) | ||
| ) | ||
| expect_true(possibly_in_proj(child)) | ||
| expect_false(is_package(child)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until now, we had never actually intentionally created a nested package or project in the tests 🤷♀️
| on.exit({ | ||
| setwd(oldwd) | ||
| dir_delete(newdir) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated cleanup.
|
Not in this PR and already in |
| parent <- scoped_temporary_package() | ||
| with_mock( | ||
| # since user can't approve interactively, use the backdoor | ||
| `usethis:::allow_nested_project` = function() TRUE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this just be allow_nested_project = ...?
Related to the "Can we remove
is_testing()too?" comment in #1084 (review)This removes
is_testing()and does a general tidy up around nested projects.