Extract test framework #793

GeorgeHahn · 2020-07-10T02:20:46Z

Motivation

A published crate with mocks for common tracing types will benefit the greater tracing ecosystem.

Solution

Extract the existing mocks into a separate crate. I did some minor refactoring to (hopefully) improve clarity and decrease verbosity. These are in standalone commits to make it easy to pick and choose changes. (The exceptions are fixup commits, which should be dropped if the previous commit(s) are dropped.)

I'm not sure if no-std is useful for this and I didn't test it, but I left the scaffolding in place for it.

From issue #539

GeorgeHahn · 2020-07-10T03:06:48Z

Sorry for the force pushes, I had a couple of small errors to fix and wanted to preserve the single commit per refactoring. I won't rebase future changes.

hawkw · 2020-07-10T17:31:47Z

@GeorgeHahn don't worry about it, that's fine!

davidbarsky

I don't really have that much feedback given that these changes were largely mechanical, so if my concerns are addressed I'm happy to get this merged.

I also hope that you used some automated refactoring tools instead of doing all this by hand!

tracing-test/src/lib.rs

davidbarsky · 2020-07-26T19:05:25Z

tracing-test/src/lib.rs

+//! ```rust
+//! # fn docs() {
+//! // TODO
+//! # }
+//! ```


Can you add an end-to-end example of something like the event_with_message test in tracing?

Done. I added a span to show the enter/exit/drop sequence, but I'm not sure if it's useful enough to justify the added lines. What do you think?

tracing-attributes/tests/fields.rs

GeorgeHahn · 2020-08-01T04:25:53Z

Thanks for the review! Sorry it took so long for me to revisit this.

I also hope that you used some automated refactoring tools instead of doing all this by hand!

Fortunately yes! 😄 This was mostly done with regexes, though rust-analyzer is at the point where it could have handled a lot of these changes too.

hawkw · 2020-08-12T18:36:29Z

We should update this branch to integrate the changes to the mock subscribers from PR #908 (adding support for testing with the max level hint). Also, PR #919 will make additional changes to the mock subscribers, so we'll want to integrate that as well.

hawkw

Thanks for working on this, and I'm sorry it took me so long to look at it.

Overall, it looks good. I left a few comments, most of which are non-blocking nits. The only issue that I think needs to be addressed before this PR merges is the conditional no-std support, which I don't think will actually work as expected. We should either fix it or remove it.

hawkw · 2020-07-10T17:34:43Z

tracing-test/README.md

@@ -0,0 +1,43 @@
+# tracing-test
+
+Testing utilities for the tracing ecosystem


Nit: since the README is rendered for the crate's description page on crates.io, I usually like to make sure there is a link back to tracing's crates.io page somewhere near the top.

hawkw · 2020-07-10T17:35:19Z

tracing-test/src/lib.rs

@@ -0,0 +1,66 @@
+//! Testing utilities for `tracing`


nit: similarly, nice to link to tracing on docs.rs here.

hawkw · 2020-08-12T18:38:28Z

tracing-test/src/span.rs

-            },
-        }
-    }
+pub fn new() -> MockSpan {


WDYT about naming this something like any? Then, the test DSL would look like this:

span::any() .at_level(...) .with_field(...)

hawkw · 2020-08-12T18:41:26Z

tracing/tests/event.rs

+                    event().with_fields(
+                        field("answer")
                            .with_value(&42)
                            .and(
-                                field::mock("to_question")
+                                field("to_question")
                                    .with_value(&"life, the universe, and everything"),
                            )
                            .only(),


it might be worth reworking this API a bit to reduce the rightward drift from multiple fields, like

event() .with_field(field("answer").with_value(&42)) .with_field(field('to_question").with_value(&"life, the universe, and everything")) .only()

but, that would be more than a simple mechanical change, so take it or leave it.

We could also do this in a follow-up PR, but we'd want to make the change before releasing the crate, because it would be a breaking change.

hawkw · 2020-08-12T18:42:44Z

tracing-test/src/lib.rs

+//!
+//! `tracing-test` offers mock implementations of `tracing` types such as Spans, Events,
+//! and Subscribers. These mocks are intended to be used in testing `tracing`-based crates.
+//!


Let's make sure the stability note is present in the lib.rs docs as well.

hawkw · 2020-08-12T18:43:14Z

tracing-test/README.md

+`tracing-test` provides testing utilities that are used internally in the
+`tracing` codebase and may be useful elsewhere in the `tracing` ecosystem.
+
+This crate is unstable and does not make any stability guarantees.


I think we should guarantee that we will follow semver when breaking the test support code, but not that we won't churn versions rapidly.

hawkw · 2020-08-12T18:44:35Z

tracing-test/README.md

+
+## License
+
+This project is licensed under the [MIT license](LICENSE).


Let's also add a copy of the MIT license in the tracing-test dir. Since cargo publish is run in that directory, it will only include a copy of the license if we add one there (see #842).

hawkw · 2020-08-12T18:50:37Z

tracing-test/src/lib.rs

+//! handle.assert_finished();
+//! # }
+//! ```
+#![cfg_attr(not(feature = "std"), no_std)]


I don't know if it matters all that much to have no-std support for this crate...in most cases, I think tests are run with std even in projects that are otherwise no-std, with

#[cfg(test)] extern crate std;

or similar.

On the other hand, I don't think we use any std APIs that aren't in alloc, so we could support no-std. However, I think we should make sure that this actually builds without std, before publishing it with no-std.

For example, I think that the import paths for the std APIs we use won't work, since they might be coming from core/alloc. If we want the test support code to support no-std, we need to do something like what tracing and tracing-core do: create a module that conditionally re-exports from either std or core/alloc and ensure it's used everywhere instead of std.

mihai-dinculescu · 2020-10-03T12:50:42Z

Any update on this? Would love to see it merged :)

GeorgeHahn · 2020-10-03T14:12:57Z

Sorry, I've been super busy with life stuff. This is still on my mind, but it's probably super stale at this point. I can revisit it in a couple of weeks once I'm moved into my new home.

…

On Sat, Oct 3, 2020, 06:50 Mihai Dinculescu ***@***.***> wrote: Any update on this? Would love to see it merged :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#793 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD2XW2VS4OFHNHOD5XPORTSI4M23ANCNFSM4OWFCRDQ> .

GeorgeHahn · 2021-05-30T04:46:04Z

Closing this because it's quite stale.

GeorgeHahn added 2 commits July 9, 2020 21:16

Initial extraction of test framework

ddf0a38

Shorten MockSpan creation to span::mock("foo")

bcbf5f9

GeorgeHahn requested review from hawkw and a team as code owners July 10, 2020 02:20

GeorgeHahn force-pushed the extract-test-framework branch 2 times, most recently from ae68425 to 8cd5450 Compare July 10, 2020 02:43

GeorgeHahn added 6 commits July 9, 2020 21:56

Shorten MockSpan creation to span("foo")

2cefe46

fixup: Add anonymous_span() for unnamed mock spans

1e82c12

Rename subscriber::mock() to subscriber::expect()

634372f

Add subscriber helper: expect_nothing()

2b178ca

rename fields::mock() to fields::field()

70331f8

Rename event::mock() to event()

9738aef

GeorgeHahn force-pushed the extract-test-framework branch from 8cd5450 to 9738aef Compare July 10, 2020 02:57

davidbarsky reviewed Jul 26, 2020

View reviewed changes

LegNeato mentioned this pull request Jul 29, 2020

Add tracing support graphql-rust/juniper#720

Open

GeorgeHahn added 3 commits July 31, 2020 22:38

Improve overview doc

4081e9e

anonymous_span() -> span::new()

051cf0b

Add mock subscriber example

05430e8

hawkw requested changes Aug 12, 2020

View reviewed changes

GeorgeHahn closed this May 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract test framework #793

Extract test framework #793

GeorgeHahn commented Jul 10, 2020

GeorgeHahn commented Jul 10, 2020

hawkw commented Jul 10, 2020

davidbarsky left a comment

davidbarsky Jul 26, 2020

GeorgeHahn Aug 1, 2020

GeorgeHahn commented Aug 1, 2020

hawkw commented Aug 12, 2020

hawkw left a comment

hawkw Jul 10, 2020

hawkw Jul 10, 2020

hawkw Aug 12, 2020

hawkw Aug 12, 2020

hawkw Aug 12, 2020

hawkw Aug 12, 2020

hawkw Aug 12, 2020

hawkw Aug 12, 2020

mihai-dinculescu commented Oct 3, 2020

GeorgeHahn commented Oct 3, 2020 via email

GeorgeHahn commented May 30, 2021

		@@ -0,0 +1,43 @@
		# tracing-test

		Testing utilities for the tracing ecosystem


		## License

		This project is licensed under the [MIT license](LICENSE).

Extract test framework #793

Extract test framework #793

Conversation

GeorgeHahn commented Jul 10, 2020

Motivation

Solution

GeorgeHahn commented Jul 10, 2020

hawkw commented Jul 10, 2020

davidbarsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeorgeHahn commented Aug 1, 2020

hawkw commented Aug 12, 2020

hawkw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihai-dinculescu commented Oct 3, 2020

GeorgeHahn commented Oct 3, 2020 via email

GeorgeHahn commented May 30, 2021