Skip to content

split 'compile_error' toy into many test cases, demonstrate new issues #134

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

Merged
merged 1 commit into from
Apr 18, 2025

Conversation

BenWiederhake
Copy link
Contributor

This PR implements a proper compile-test framework as suggested in #133 (review):

I think a proper compile-test framework sounds pretty good for this crate, because it is so focused on derive-based functionality.

This basically splits examples/test_compile_errors_manually.rs into many testcases that can run automatically. "arguments_file_writeonly" was a bit tricky, since it needs a write-only file, and I don't want to touch on the subject of having to do cleanup after testing.

The *.stderr-files are semi-automatically generated, not hand-crafted, and will need to be updated whenever Rust changes it's error formatting, or whenever we change something about our error messages.

Out of scope for this PR:

  • actually improving the error message to use syn-spans (because I first want to get agreement on how to do the automated tests)
  • fixing the newly found issues

@BenWiederhake
Copy link
Contributor Author

Build failure is due to crate derive checking for the complete feature in the wrong way. This is solved by #131, so let's merge it (or #132 if you insist).

@BenWiederhake BenWiederhake marked this pull request as draft April 15, 2025 01:00
@BenWiederhake BenWiederhake marked this pull request as ready for review April 15, 2025 21:07
@BenWiederhake
Copy link
Contributor Author

Changes since last push:

@tertsdiepraam
Copy link
Member

Oh I have another question. Should we fix these tests to a specific Rust version in case rustc changes their error format?

@BenWiederhake
Copy link
Contributor Author

Changes since last push:

  • rebased on current main
  • Changed the arg_missing_metavar test such that the type does not impl Default. So you're absolutely right, thanks @tertsdiepraam! :)

I don't think it's very likely that rustc will change the error messages around these things, since we don't use fancy new features that are likely to change. On the other hand, I don't have enough experience with these things to really know for sure how stable Rust's error messages are. Some points I see are:

  • There are new Rust versions constantly. So if we fix it to a particular version, that means we will have to update that piece of information every other 6 weeks or so. That sounds like a lot of pointless work.
  • If the output changes, the test will fail, and we're the first to discover that the error messages have changed (and potentially become useless). We'll have ample opportunity to disable or adjust the tests, and/or start implementing the version-fixing. This might even happen before a user runs into one of the (changed, possibly useless) new error messages. I think this is good.

So I suggest we don't do any version fixing right now.

@tertsdiepraam tertsdiepraam merged commit c2b91a7 into uutils:main Apr 18, 2025
4 checks passed
@BenWiederhake BenWiederhake deleted the dev-derive-trybuild branch April 19, 2025 19:55
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