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

Change compile requirements for running cargo test #233

Closed
YeungOnion opened this issue May 25, 2024 · 3 comments · Fixed by #234
Closed

Change compile requirements for running cargo test #233

YeungOnion opened this issue May 25, 2024 · 3 comments · Fixed by #234

Comments

@YeungOnion
Copy link
Contributor

Do we need the nightly compiler just for the test suite? The requirement is for feature fn_traits (I believe this requires feature unboxed_closures) is from statrs::testing_boiler but nowhere else.

Feedback seems to be,

  • non-obvious to test with nightly for new contributors (even when familiar with rust projects)
  • assertion error messages reference call to macro statrs::testing_boiler, not clear what line test is defined on.

Open to other feedback, but what I'm imagining is a few proc macros that expand to unwraps or assertions

  • create(dist_family_name, tuple_of_dist_parameters, ...error msg)
  • bad_create(... sim to previous asserting is_err
  • test_almost(dist_family_name, tuple_of_dist_parameters, dot_method_call_with_args, ...error_msg)
@FreezyLemon
Copy link
Contributor

Related: #232

I'm pretty sure the testing_boiler macro can be changed so that it doesn't need any nightly compiler features.

I wonder about the second part. Does RUST_BACKTRACE=1 not give any useful information? How would an optional error message, e.g. in test_almost(/* .. */, "error message here"), help with test suites like these? There must be a better way than adding error messages "line 1", "line 2", ..., right?

@YeungOnion
Copy link
Contributor Author

RUST_BACKTRACE=1 does provide useful info, perhaps I'm a little silly. I think the messages would be convenient, but I think changing what we have for existing would be better.

@YeungOnion
Copy link
Contributor Author

resolved by #234

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 a pull request may close this issue.

2 participants