-
Notifications
You must be signed in to change notification settings - Fork 295
transpile: tests: add .expect_unresolved_import instead of hardcoding libc
#1719
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,14 @@ | ||
| use itertools::Itertools; | ||
| use log::warn; | ||
| use std::collections::HashSet; | ||
| use std::fmt; | ||
| use std::fmt::Display; | ||
| use std::fmt::Formatter; | ||
| use std::io; | ||
| use std::io::Write; | ||
| use std::path::Path; | ||
| use std::process::Command; | ||
| use std::str; | ||
| use std::str::FromStr; | ||
|
|
||
| use crate::RustEdition::Edition2021; | ||
|
|
@@ -146,6 +150,7 @@ pub struct Rustc<'a> { | |
| edition: RustEdition, | ||
| crate_name: Option<&'a str>, | ||
| expect_error: bool, | ||
| imported_crates: &'a [&'a str], | ||
| } | ||
|
|
||
| pub fn rustc(rs_path: &Path) -> Rustc { | ||
|
|
@@ -154,6 +159,7 @@ pub fn rustc(rs_path: &Path) -> Rustc { | |
| edition: Default::default(), | ||
| crate_name: None, | ||
| expect_error: false, | ||
| imported_crates: Default::default(), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -176,20 +182,39 @@ impl<'a> Rustc<'a> { | |
| } | ||
| } | ||
|
|
||
| pub fn expect_unresolved_imports(self, imported_crates: &'a [&'a str]) -> Self { | ||
| Self { | ||
| imported_crates, | ||
| ..self | ||
| } | ||
| } | ||
|
|
||
| pub fn run(self) { | ||
| let Self { | ||
| rs_path, | ||
| edition, | ||
| crate_name, | ||
| expect_error, | ||
| imported_crates, | ||
| } = self; | ||
| let crate_name = | ||
| crate_name.unwrap_or_else(|| rs_path.file_stem().unwrap().to_str().unwrap()); | ||
| run_rustc(rs_path, edition, crate_name, expect_error); | ||
| run_rustc(rs_path, edition, crate_name, expect_error, &imported_crates); | ||
| } | ||
| } | ||
|
|
||
| fn run_rustc(rs_path: &Path, edition: RustEdition, crate_name: &str, expect_error: bool) { | ||
| fn run_rustc( | ||
| rs_path: &Path, | ||
| edition: RustEdition, | ||
| crate_name: &str, | ||
| expect_error: bool, | ||
| imported_crates: &[&str], | ||
| ) { | ||
| let rs = fs_err::read_to_string(rs_path).unwrap(); | ||
| for imported_crate in imported_crates { | ||
| assert!(rs.contains(&format!("::{imported_crate}"))); | ||
| } | ||
|
|
||
| // There's no good way to not create an output with `rustc`, | ||
| // so just create an `.rlib` and then delete it immediately. | ||
| let rlib_path = rs_path.with_file_name(format!("lib{crate_name}.rlib")); | ||
|
|
@@ -206,10 +231,59 @@ fn run_rustc(rs_path: &Path, edition: RustEdition, crate_name: &str, expect_erro | |
| "-o", | ||
| ]); | ||
| cmd.args([&rlib_path, rs_path]); | ||
| let status = cmd.status().unwrap(); | ||
| let output = cmd.output().unwrap(); | ||
| let status = output.status; | ||
| if status.success() { | ||
| fs_err::remove_file(&rlib_path).unwrap(); | ||
| } | ||
| io::stdout().write_all(&output.stdout).unwrap(); | ||
| io::stderr().write_all(&output.stderr).unwrap(); | ||
|
|
||
| // Using rustc itself to build snapshots that reference crates (like libc) is difficult because we don't know | ||
| // the appropriate --extern libc=/path/to/liblibc-XXXXXXXXXXXXXXXX.rlib to pass. | ||
| // Skip for now, as we've already compared the literal text, | ||
| // and we're checking the error messages here. | ||
| let stderr = str::from_utf8(&output.stderr).unwrap(); | ||
| let mut error_lines = stderr | ||
| .split('\n') | ||
| // .split(|&b| b == b'\n') | ||
| .filter(|line| line.starts_with("error[E")) | ||
| .collect::<HashSet<_>>(); | ||
| dbg!(&error_lines); | ||
| for imported_crate in imported_crates { | ||
| // For `::{imported_crate}::*`. | ||
| let absolute_path = match edition { | ||
| Edition2021 => format!("error[E0433]: failed to resolve: could not find `{imported_crate}` in the list of imported crates"), | ||
| Edition2024 => format!("error[E0433]: cannot find `{imported_crate}` in the crate root"), | ||
| }; | ||
|
Comment on lines
+256
to
+258
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to me that we should parse the error code from the message and filter on that plus the mentioned library name. Hard-coding the whole error message seems very brittle.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do I parse the library name, though, without hardcoding the error message? I agree it could be brittle, but it'd have a simple error and then we update the error message needed, so it's a simple fix if it's ever changed, too.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the library name we do probably have to choose between something brittle (like hard-coding the error message) and something loose (like a simple string-contains check), but for the error code we should be able to parse in a somewhat robust way. E.g., in JSON output format it's given in its own "code" field. Even without JSON output, the "error[ENNNN]" prefix should be relatively stable. |
||
| // For `use ::{imported_crate}*`. | ||
| let absolute_use_path = format!("error[E0432]: unresolved import `{imported_crate}`"); | ||
| // For `{imported_crate}::*`. | ||
| let relative_path = match edition { | ||
| Edition2021 => format!("error[E0433]: failed to resolve: use of undeclared crate or module `{imported_crate}`"), | ||
| Edition2024 => format!("error[E0433]: cannot find module or crate `{imported_crate}` in this scope"), | ||
| }; | ||
|
|
||
| let absolute_path = absolute_path.as_str(); | ||
| let absolute_use_path = absolute_use_path.as_str(); | ||
| let relative_path = relative_path.as_str(); | ||
| dbg!(absolute_path); | ||
| dbg!(absolute_use_path); | ||
| dbg!(relative_path); | ||
|
|
||
| // Pre-compute to avoid `||` short-circuiting. | ||
| let absolute_path = error_lines.remove(absolute_path); | ||
| let absolute_use_path = error_lines.remove(absolute_use_path); | ||
| let relative_path = error_lines.remove(relative_path); | ||
|
|
||
| assert!(absolute_path || absolute_use_path || relative_path); | ||
| } | ||
| if !imported_crates.is_empty() { | ||
| dbg!(&error_lines); | ||
| assert!(error_lines.is_empty()); | ||
| return; | ||
| } | ||
|
|
||
| if expect_error { | ||
| assert!( | ||
| !status.success(), | ||
|
|
||
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.
This comment ("skip for now") doesn't make sense anymore as compilation is no longer being skipped.