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

run-make-support: add helpers for fs operations #125728

Closed
jieyouxu opened this issue May 29, 2024 · 1 comment · Fixed by #125736
Closed

run-make-support: add helpers for fs operations #125728

jieyouxu opened this issue May 29, 2024 · 1 comment · Fixed by #125736
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

jieyouxu commented May 29, 2024

Noticed in #125683 (comment), we should add helpers for:

  • fs operation wrappers that panic on failure and provide good context to aid debugging
    • e.g. wrap std::fs::remove_file(path) with run_make_support::fs::remove_file(path) but on panic shows the path we're trying to remove, so the test writer doesn't forget to check the result of fs operations.

cc @Oneirical in case you want to work on this.

UPDATE: we reworked rmake.rs tests to not run in the test sources directory so tmp_dir().join() is no longer relevant.

@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 29, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 29, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 29, 2024
@jieyouxu
Copy link
Member Author

jieyouxu commented May 29, 2024

Basically we want the support library to provide APIs almost like https://docs.rs/fs-err/latest/fs_err/ except panic-on-failure with good error messages instead of returning a Result.

bors added a commit to rust-lang-ci/rust that referenced this issue Jun 6, 2024
…r=jieyouxu

run-make-support: add helpers for `tmp_dir().join()` and `fs` operations

Suggested by rust-lang#125728.

The point of this wrapper is to stop silent fails caused by forgetting to `unwrap` `fs` functions. However, functions like `fs::read` which return something and get stored in a variable should cause a failure on their own if they are not unwrapped (as the `Result` will be stored in the variable, and something will be done on that `Result` that should have been done to its contents). Is it still pertinent to wrap `fs::read_to_string`, `fs::metadata` and so on?

try-job: x86_64-msvc
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 6, 2024
…r=<try>

run-make-support: add helpers for `tmp_dir().join()` and `fs` operations

Suggested by rust-lang#125728.

The point of this wrapper is to stop silent fails caused by forgetting to `unwrap` `fs` functions. However, functions like `fs::read` which return something and get stored in a variable should cause a failure on their own if they are not unwrapped (as the `Result` will be stored in the variable, and something will be done on that `Result` that should have been done to its contents). Is it still pertinent to wrap `fs::read_to_string`, `fs::metadata` and so on?

try-job: x86_64-msvc
try-job: i686-mingw
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 6, 2024
…r=<try>

run-make-support: add helpers for `tmp_dir().join()` and `fs` operations

Suggested by rust-lang#125728.

The point of this wrapper is to stop silent fails caused by forgetting to `unwrap` `fs` functions. However, functions like `fs::read` which return something and get stored in a variable should cause a failure on their own if they are not unwrapped (as the `Result` will be stored in the variable, and something will be done on that `Result` that should have been done to its contents). Is it still pertinent to wrap `fs::read_to_string`, `fs::metadata` and so on?

try-job: x86_64-msvc
try-job: i686-mingw
@jieyouxu jieyouxu changed the title run-make-support: add helpers for tmp_dir().join() and fs operations run-make-support: add helpers for fs operations Jun 8, 2024
@jieyouxu jieyouxu added the A-run-make Area: port run-make Makefiles to rmake.rs label Jun 9, 2024
@bors bors closed this as completed in 3ea5e23 Jun 11, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jun 13, 2024
run-make-support: add wrapper for `fs` operations

Suggested by #125728.

The point of this wrapper is to stop silent fails caused by forgetting to `unwrap` `fs` functions. However, functions like `fs::read` which return something and get stored in a variable should cause a failure on their own if they are not unwrapped (as the `Result` will be stored in the variable, and something will be done on that `Result` that should have been done to its contents). Is it still pertinent to wrap `fs::read_to_string`, `fs::metadata` and so on?

Closes: rust-lang/rust#125728

try-job: x86_64-msvc
try-job: i686-mingw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants