Add import layers for testthat files#1260
Conversation
lionel-
force-pushed
the
oak/salsa-21-multi-target
branch
from
3bba52f to
10b4801
Compare
lionel-
force-pushed
the
oak/salsa-21-multi-target
branch
from
10b4801 to
a6cc84b
Compare
lionel-
force-pushed
the
oak/salsa-22-testthat
branch
from
464855a to
0e4fa01
Compare
| .as_url() | ||
| .path_segments()? | ||
| .next_back() | ||
| .filter(|seg| !seg.is_empty())?; |
There was a problem hiding this comment.
is this because https://example.com/foo/ sees that last / and returns an empty string for the (not really there) segment after the /?
| .path_segments()? | ||
| .next_back() | ||
| .filter(|seg| !seg.is_empty())?; | ||
| Some(percent_decode_str(last).decode_utf8_lossy()) |
There was a problem hiding this comment.
are you sure you dont want decode_utf8().log_err()? given that this already returns an option?
| // at any offset. Only the file's own top-level `library()` calls | ||
| // narrow. |
There was a problem hiding this comment.
"Only the file's own top-level library() calls"
GROSSSSSSSS, THE HORROR
| /// order (latest-attached first). `before` narrows to a top-level cursor: with | ||
| /// `Some(offset)`, keep only file-scope calls that have run by `offset`; with | ||
| /// `None`, keep every attach (the end-of-file view, used for lazy contexts). |
There was a problem hiding this comment.
is the cursor itself top level? not always right? like you can be in a test_that({<cursor>}) block and i wouldnt consider that top level?
i would also appreciate if this comment were rewritten as like a bulleted list of the 2 cases. it was very hard for me to mentally parse with the colon and semicolon
| /// 6. base. | ||
| /// | ||
| /// `offset` narrows the components of layer 4. `None` produces the end-of-file | ||
| /// view and keeps every `library()` call. `Some(offset)` keeps only the calls |
There was a problem hiding this comment.
| /// view and keeps every `library()` call. `Some(offset)` keeps only the calls | |
| /// view and utilizes every `library()` call. `Some(offset)` utilizes only the calls |
"keep" felt like a weird verb here
| fn in_testthat_dir(path: &Utf8Path) -> bool { | ||
| let Some(parent) = path.parent() else { | ||
| return false; | ||
| }; | ||
| parent.file_name() == Some("testthat") && | ||
| parent.parent().and_then(Utf8Path::file_name) == Some("tests") | ||
| } |
There was a problem hiding this comment.
we should upgrade to 2024 edition rust at some point for let chains so you can do things like this
fn in_testthat_dir(path: &Utf8Path) -> bool {
if let Some(parent) = path.parent() &&
parent.file_name() == Some("testthat") &&
let Some(parent) = parent.parent() &&
parent.file_name() == Some("tests")
{
return true
} else {
return false
}
}| /// which currently sorts based on locale, but arguably this should be fixed on | ||
| /// the testthat side. | ||
| fn testthat_support_key(file: File, db: &dyn Db) -> Cow<'_, str> { | ||
| file.path(db).file_name().unwrap_or_default() |
There was a problem hiding this comment.
What is the default? An empty Owned string i guess? can you just leave it as Option<Cow>? I have no idea if that would sort right
| pkg.set_scripts(&mut db) | ||
| .to(vec![helper, setup, test_foo, test_bar]); |
There was a problem hiding this comment.
i wonder if it will be useful to have a specific field for testthat_scripts or something, so we dont have to tease them apart from regular scripts every time
2 participants
Branched from #1259
Whereas scripts in
tests/folders are standalone, scripts intests/testthat/are sourced in an environment where:testthatexports are attachedThis PR detects testthat files and adjusts the import layers accordingly, so that symbols injected by testthat are visible in these files.