Move alloc tests to rmake #121918
Move alloc tests to rmake #121918
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
rustbot
added
A-testsuite
| use std::path::PathBuf; | ||
|
|
||
| fn main() { | ||
| let lib_path = PathBuf::from("../../../library/alloc/src/lib.rs"); |
There was a problem hiding this comment.
You can avoid path separators (PathBuf::from does not attempt to validate its string source, nor does it convert path separators AFAIK) via:
let lib_path = ["..", "..", "..", "library", "alloc", "src", "lib.rs"].iter().collect();This would internally use the platform separator for the target the test is running on, and when you do path.to_str().unwrap() you would get back a path string with the correct platform separator, for any executable (including the ones that don't try to normalize paths).
We probably want to add some arg_path convenience API that does this, but I'm not sure of the exact API / semantics and potential footguns.
There was a problem hiding this comment.
Should I also try to use the absolute path here using tmp dir i think or the relative path is okay?
There was a problem hiding this comment.
Here tmpdir is irrelevant because we're actually referring to the stdlib sources, not compiled libs, so relative path is correct
|
I added an arg path to the rustc function. or would it make more sense to add a general path function? |
|
This looks like a reasonable starting point to me, but can you squash commits down into 1? |
abhay-51
force-pushed
the
Move_alloc_tests_to_rmake
branch
from
0a4502a
to
1e6d860
Compare
|
@Mark-Simulacrum I have squashed the commits. Lmk if I need to make any other changes. Thanks!! |
|
☔ The latest upstream changes (presumably #122036) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Could you please also add a comment documenting the intent of the test? |
|
Needs rebase + addressing jieyouxu's comment. @rustbot author |
rustbot
added
S-waiting-on-author
Rewrite `core-no-fp-fmt-parse` test in Rust Claiming the simple "core-no-fp-fmt-parse" test from rust-lang#121876. `run_make_support` was altered with `arg_path` written in rust-lang#121918 by `@abhay-51,` with additional doc comment. Preliminary GSoC contribution for the project proposal mentored by `@jieyouxu.`
Rollup merge of rust-lang#123180 - Oneirical:master, r=Mark-Simulacrum Rewrite `core-no-fp-fmt-parse` test in Rust Claiming the simple "core-no-fp-fmt-parse" test from rust-lang#121876. `run_make_support` was altered with `arg_path` written in rust-lang#121918 by `@abhay-51,` with additional doc comment. Preliminary GSoC contribution for the project proposal mentored by `@jieyouxu.`
|
@abhay-51 any updates on this? |
Rewrite 3 very similar `run-make` alloc tests to rmake Part of rust-lang#121876 rust-lang#121918 attempted to port these 3 tests 2 months ago. However, since then, the structure of `run-make-support` has changed a bit and new helper functions were added. Since there has been no activity on the PR, they are good low-hanging fruit to knock down, using the new functions of the current library. There is also the removal of a useless import on a very similar test.
|
I'm closing this since it has been handled by #125024, which has been merged. |
Issues from the tracking PR #121876
cc @jieyouxu