Skip to content

transpile: tests: add .expect_unresolved_import instead of hardcoding libc#1719

Open
kkysen wants to merge 3 commits intomasterfrom
kkysen/transpile-snapshots-expect_unresolved_import
Open

transpile: tests: add .expect_unresolved_import instead of hardcoding libc#1719
kkysen wants to merge 3 commits intomasterfrom
kkysen/transpile-snapshots-expect_unresolved_import

Conversation

@kkysen
Copy link
Copy Markdown
Contributor

@kkysen kkysen commented Apr 1, 2026

Also, we check that the error messages are as expected, too (meaning we actually run rustc). This also makes the libc tests more explicit.

This should also handle #1716.

kkysen added 3 commits April 1, 2026 12:52
…ng `libc`

Also, we check that the error messages are as expected, too (meaning we actually run `rustc`).
This also makes the `libc` tests more explicit.
@kkysen kkysen force-pushed the kkysen/transpile-snapshots-expect_unresolved_import branch from a52145e to becc685 Compare April 1, 2026 22:18
Base automatically changed from kkysen/transpile-snapshots-expect_error to master April 2, 2026 19:47
Copy link
Copy Markdown
Contributor

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general approach seems okay (not wonderful but Cargo is very resistant to helping us find the requisite information here), but I wasn't happy with some specifics. After addressing inline comments should be good to go.


// 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,
Copy link
Copy Markdown
Contributor

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.

Comment on lines +256 to +258
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"),
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

@fw-immunant fw-immunant Apr 13, 2026

Choose a reason for hiding this comment

The 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants