Append #[::core::prelude::v1::test] only if no test macros exist #46

kezhuw · 2024-04-19T10:03:20Z

This way we could avoid duplicated test runs if #[test] variants already exist

See also frondeus/test-case#101, frondeus/test-case#143 and tokio-rs/tokio#6497.

Closes #35.

d-e-s-o

Thanks for the proposal! I am fine with the approach. Left a few nitpicks.

This pr depends on frondeus/test-case#143 and tokio-rs/tokio#6497.

Can we please wait until these are merged? I'd rather not include any temporary patches.

macros/src/lib.rs

kezhuw · 2024-04-20T00:23:00Z

@d-e-s-o Thank you for reviewing. I am good with the merge timing.

I have pushed with a fixup commit to solve your concerns. Also, I checked that there is no duplicated test runs for new test cases. cargo test with_existing runs 2 tests, cargo test with_append runs 7 tests including ones generated by test_case.

d-e-s-o · 2025-01-13T13:54:21Z

It seems as if the tokio change has made it in and test-case can't get its act together. @kezhuw I think we use test-case only for exercising attribute parsing with special-sauce syntax. If you know of a different crate supporting similar non-standard syntax, then we can just switch existing tests away from test-case.

kezhuw · 2025-01-15T13:40:10Z

I think we use test-case only for exercising attribute parsing with special-sauce syntax.

Do you mean tests like this ?

#[test_log::test(test_case::test_case(-2, -4; "my test name"))]
fn with_inner_test_attribute_and_test_args_and_name(x: i8, y: i8) {
  assert_eq!(x, -2);
  assert_eq!(y, -4);
}

This pr does not depend on this. I can drop some tests so that we don't need to patch test-case crate.

kezhuw · 2025-01-15T14:05:36Z

I can drop some tests so that we don't need to patch test-case crate.

Those tests should pass once frondeus/test-case#143 published. Or I could #[ignore] and doc them.

d-e-s-o · 2025-01-15T16:07:24Z

Right, I understand it's just a development thing and all that, but my point is that we don't want to loose the coverage nor do we want to depend on some unpublished snapshot (or add ignore broken tests, for that matter).

So the easiest way out that I can think of would be to just use some other proc macro that supports more or less arbitrary syntax in test-case's stead. That would then unblock this PR. That it would (presumably) break test-case interaction is something I can live with, given that at this point that crate seems to be abandoned and we cannot freeze the entire eco system because of that.

kezhuw · 2025-01-16T01:42:16Z

we don't want to loose the coverage

For test coverage, the following two methods are identical in test-log side except that it will fail without patch to test-case. Besides, tokio::test also accept attributes.

#[tokio::test]
#[test_log::test]
async fn with_append_test_attribute_and_async() {
  assert_eq!(async { 42 }.await, 42)
}

#[test_case::test_case(-2, -4)]
#[test_case::test_case(-2, -5)]
#[test_log::test]
fn with_append_test_attribute_and_test_args(x: i8, _y: i8) {
  assert_eq!(x, -2);
}

So the easiest way out that I can think of would be to just use some other proc macro that supports more or less arbitrary syntax in test-case's stead.

Let me figure out.

This way test macros following `#[rstest]` can decide whether or not to generate test macro to avoid duplicate test runs. It is an attempt to improve capabilities among test macros. Currently, following test from [googletest](https://github.com/google/googletest-rust/blob/21f2948684847922a416252b8118e3eada8e29d6/integration_tests/src/google_test_with_rstest.rs#L52-L57)(`main` branch at 2025-01-16) will run twice. ```rust #[rstest] #[case(1)] #[gtest] fn paramterised_test_should_work_with_rstest_first(#[case] value: u32) -> Result<()> { verify_that!(value, eq(value)) } ``` See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.

`#[gtest]` will benefit from la10736/rstest#291, we could also benefit other test macros. It is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. We could deprecate `#[googletest::test]` oneday after rust-lang/rust#82419 released. See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.

`#[gtest]` will benefit from la10736/rstest#291, we could also benefit other test macros. It is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. We could deprecate `#[googletest::test]` oneday after la10736/rstest#291 released. See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.

`#[gtest]` will benefit from la10736/rstest#291, we could also benefit other test macros. This is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. We could deprecate `#[googletest::test]` oneday after la10736/rstest#291 released. See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.

This is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. Without the fix, the new test will run twice. See also tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, google/googletest-rust#561, kezhuw/stuck#53.

kezhuw · 2025-06-28T12:10:48Z

Hi @d-e-s-o, would you mind take another look ? I have replaced test-case with rstest in commit 628dff9 and rebased this pr to main.

d-e-s-o

I think this looks great, thanks! Left a few more comments. Your initial PR description mentioned that we depend on frondeus/test-case#143. Why is that no longer the case? Unfortunately somehow GitHub seems to have managed to loose the original set of changes... :-(

d-e-s-o · 2025-06-29T17:59:15Z

macros/src/lib.rs

+  const CANDIDATES_LEN: usize = 4;
+
+  let candidates: [[&str; CANDIDATES_LEN]; 2] = [
+    ["core", "prelude", "*", "test"],
+    ["std", "prelude", "*", "test"],
+  ];


Can we go with the slightly simpler version you used here?

d-e-s-o · 2025-06-29T18:01:01Z

tests/mod.rs

+#[case(-3, -4)]
+#[test_log::test]
+// Applied to all cases, must not before `rstest`, see https://github.com/la10736/rstest/issues/210
+#[should_panic] // https://docs.rs/rstest/latest/rstest/attr.rstest.html#use-specific-case-attributes


Let's use versioned URLs for the sake of persistence? https://docs.rs/rstest/0.25.0/rstest/attr.rstest.html#use-specific-case-attributes

d-e-s-o · 2025-06-29T18:01:25Z

tests/mod.rs

+#[case(-2, -4)]
+#[case(-3, -4)]
+#[test_log::test]
+// Applied to all cases, must not before `rstest`, see https://github.com/la10736/rstest/issues/210


Suggested change

// Applied to all cases, must not before `rstest`, see https://github.com/la10736/rstest/issues/210

// Applied to all cases, must not come before `rstest`, see https://github.com/la10736/rstest/issues/210

Same below.

d-e-s-o · 2025-06-29T18:02:50Z

tests/mod.rs

 use tracing::debug;
 use tracing::error;
 use tracing::info;
 use tracing::instrument;

-


Nit: Let's keep spurious changes to a minimum?

d-e-s-o · 2025-06-29T18:13:17Z

Why is that no longer the case? Unfortunately somehow GitHub seems to have managed to loose the original set of changes... :-(

I suspect it's because you added additional test-case based tests initially, but can't check. Just double checking if I am understanding correctly.

kezhuw · 2025-06-30T03:12:12Z

Here is the no rebased branch: https://github.com/d-e-s-o/test-log/commits/628dff9c4e0ea63c7e97d7f622e3ac89129169bf/

I think we are here because of #46 (comment).

d-e-s-o

Looks good to me. Feel free to squash the fixup commit and then we can get it in. Thanks again for your work.

This way we could avoid duplicated test runs if `#[test]` variants already exist See also frondeus/test-case#101, frondeus/test-case#143 and tokio-rs/tokio#6497. Closes d-e-s-o#35.

kezhuw · 2025-06-30T05:36:59Z

Squashed. @d-e-s-o Thank you for reviewing!

Add a CHANGELOG entry for pull request #46, which improved cooperation with other procedural macros also using #[test] attributes in some way and allowing them to be stacked.

With pull request #46 merged, we now have better support for stacking other #[test] attributes on top of ours. Document that better.

d-e-s-o approved these changes Apr 19, 2024

View reviewed changes

macros/src/lib.rs Outdated Show resolved Hide resolved

macros/src/lib.rs Outdated Show resolved Hide resolved

macros/src/lib.rs Outdated Show resolved Hide resolved

d-e-s-o mentioned this pull request May 18, 2024

test is started 2 times using #[tokio::test] #51

Closed

kezhuw mentioned this pull request Jan 16, 2025

Append generated test macro so next test macros are aware of it la10736/rstest#291

Merged

kezhuw mentioned this pull request Jan 20, 2025

Append generated test macro so next test macros are aware of it google/googletest-rust#561

Merged

kezhuw mentioned this pull request Apr 1, 2025

Append generated test macro so next test macros are aware of it frozenlib/test-strategy#17

Merged

d-e-s-o mentioned this pull request May 9, 2025

add support for rstest #58

Closed

kezhuw force-pushed the test-proc-macros-cooperation branch from 628dff9 to fb4f8d8 Compare June 28, 2025 12:05

kezhuw changed the title ~~fix: append #[::core::prelude::v1::test] only if it does not exist~~ Append #[::core::prelude::v1::test] only if no test macros exist Jun 28, 2025

kezhuw force-pushed the test-proc-macros-cooperation branch from fb4f8d8 to 1ffb4ee Compare June 29, 2025 17:09

d-e-s-o reviewed Jun 29, 2025

View reviewed changes

d-e-s-o approved these changes Jun 30, 2025

View reviewed changes

Append #[::core::prelude::v1::test] only if no test macros exist

c6ccb6c

This way we could avoid duplicated test runs if `#[test]` variants already exist See also frondeus/test-case#101, frondeus/test-case#143 and tokio-rs/tokio#6497. Closes d-e-s-o#35.

kezhuw force-pushed the test-proc-macros-cooperation branch from c45420f to c6ccb6c Compare June 30, 2025 05:34

d-e-s-o merged commit d0ac712 into d-e-s-o:main Jun 30, 2025
6 checks passed

d-e-s-o added a commit that referenced this pull request Jun 30, 2025

Add CHANGELOG entry for #46

634e9e6

Add a CHANGELOG entry for pull request #46, which improved cooperation with other procedural macros also using #[test] attributes in some way and allowing them to be stacked.

d-e-s-o added a commit that referenced this pull request Jun 30, 2025

Document attribute stacking better

04a9a41

With pull request #46 merged, we now have better support for stacking other #[test] attributes on top of ours. Document that better.

d-e-s-o added a commit that referenced this pull request Jun 30, 2025

Document attribute stacking better

ed1b74b

With pull request #46 merged, we now have better support for stacking other #[test] attributes on top of ours. Document that better.

	// Applied to all cases, must not before `rstest`, see https://github.com/la10736/rstest/issues/210
	// Applied to all cases, must not come before `rstest`, see https://github.com/la10736/rstest/issues/210

Append #[::core::prelude::v1::test] only if no test macros exist #46

Append #[::core::prelude::v1::test] only if no test macros exist #46

Uh oh!

Conversation

kezhuw commented Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d-e-s-o left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kezhuw commented Apr 20, 2024

Uh oh!

d-e-s-o commented Jan 13, 2025

Uh oh!

kezhuw commented Jan 15, 2025

Uh oh!

kezhuw commented Jan 15, 2025

Uh oh!

d-e-s-o commented Jan 15, 2025

Uh oh!

kezhuw commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kezhuw commented Jun 28, 2025

Uh oh!

d-e-s-o left a comment

Choose a reason for hiding this comment

Uh oh!

d-e-s-o Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

d-e-s-o Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

d-e-s-o Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

d-e-s-o Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

d-e-s-o Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

d-e-s-o commented Jun 29, 2025

Uh oh!

kezhuw commented Jun 30, 2025

Uh oh!

d-e-s-o left a comment

Choose a reason for hiding this comment

Uh oh!

kezhuw commented Jun 30, 2025

Uh oh!

Uh oh!

Uh oh!

kezhuw commented Apr 19, 2024 •

edited

Loading

kezhuw commented Jan 16, 2025 •

edited

Loading