Use `doc-comment` crate to run readme doctests

[casey]

Delete tests/readme.rs in favor of using the doc-comment crate to run the doctests in README.md directly.

I noticed #24, and it seems that cfg(doctest) is now stable, so I thought I'd go ahead and implement it.

[casey]

I think this is broken as written. The doctests in the readme don't appear to be running, I think because they have #[test] annotations, and the doctest wrapper itself has a #[test] annotation, so they're being nested. Taking a look now.

[casey]

Fixed, I think. I removed #[test] annotations and the fn main wrappers. The doc tests aren't smart enough to figure out the return type they should give the generated main function, so I had to add type-annotated Ok values that look like Ok::<(), bendy::serde::Error>(()).

[casey]

@thequux Ping!

[0ndorio]

Hey @casey I tested a few things based on your commit and realized we might miss some changes in here:

• One of the tests introduced through the README heavily depends on the presence of the serde feature. As we can't ensure this feature is always present when cargo test runs, it is necessary to disable this test as soon the feature is missing. This should be possible by encapsulating the content of the example in a hidden feature flagged module.

• The doctest attribute was stabilized in rustc v1.40, but we claim to support version until v1.36. Also also the test suite contains runs based on the toolchains of v1.36 and v1.38. Either we bump the MSRV to 1.40 or we add a dependency like rustversion. Using rustversion would allow us to hide the doctest in a module, only availble for compiler versions >= 1.40 on the cost of an additional runtime dependency.

Applying those changes could look like:

diff --git a/Cargo.toml b/Cargo.toml
index 185cd7c..0b87f18 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -35,9 +35,10 @@ codegen-units = 1
 failure = { version = "^0.1.3", default_features = false, features = ["derive"] }
 serde_ = { version = "^1.0" ,  optional = true, package = "serde" }
 serde_bytes = { version = "^0.11.3", optional = true }
+rustversion = "^1.0"

 [dev-dependencies]
-doc-comment = "0.3.3"
+doc-comment = "^0.3.3"
 regex = "^1.0"
 serde_derive = "^1.0"

diff --git a/README.md b/README.md
index 8b81491..b2a0381 100644
--- a/README.md
+++ b/README.md
@@ -558,6 +558,9 @@ respectively:

 ```rust
+# #[cfg(feature = "serde")]
+# mod feature_capsule {
+#
 use serde_derive::{Deserialize, Serialize};

 #[serde(crate = "serde_")]
@@ -566,17 +569,20 @@ struct Foo {
     bar: String,
 }

-let value = Foo {
-    bar: "hello".into(),
-};
+fn main() -> Result<(), bendy::serde::Error> {
+    let value = Foo {
+        bar: "hello".into(),
+    };

-let bencode = bendy::serde::to_bytes(&value)?;
-assert_eq!(bencode, b"d3:bar5:helloe");
+    let bencode = bendy::serde::to_bytes(&value)?;
+    assert_eq!(bencode, b"d3:bar5:helloe");

-let deserialized = bendy::serde::from_bytes::<Foo>(&bencode)?;
-assert_eq!(deserialized, value);
+    let deserialized = bendy::serde::from_bytes::<Foo>(&bencode)?;
+    assert_eq!(deserialized, value);

-Ok::<(), bendy::serde::Error>(())
+    Ok(())
+}
+# }

Information on how Rust types are represented in bencode is available in the diff --git a/src/lib.rs b/src/lib.rs index 22a9c7b..0c68802 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,5 +22,10 @@ pub mod serde;

pub mod value;

-#[cfg(doctest)] -doccomment::doctest!("../README.md"); +// ----------------------------------------------------------------------------- + +#[rustversion::since(1.40)] +const : () = {

• [cfg(doctest)]

• doc_comment::doctest!("../README.md"); +};

Next to this there are also two potentially intersting future features of rustc:

• Tracking issue: rfc 1990 - add external doc attribute to rustc which got recently implemented - but not yet merged - in rust-lang/rust/pull/78837. It would introduce a feature that should completly replace doc_comment.

• Tracking issue for RFC 2523, #[cfg(version(..))] which does the same for rustversion.

[casey]

@0ndorio Sounds good, and thanks for writing the diff out.

I used a module instead of #[rustversion::since(1.40)] const _ : () = { … }, but I wasn't sure if there was an advantage to using a const, so let me know if that should be changed.

[casey]

There's a formatting error on CI that I don't understand:

Error: rustfmt version (1.4.26) doesn't match the required version (1.4.22)

But the version required in rustfmt.toml is 1.4.11.

[0ndorio]

@0ndorio Sounds good, and thanks for writing the diff out.

I used a module instead of #[rustversion::since(1.40)] const _ : () = { … }, but I wasn't sure if there was an advantage to using a const, so let me know if that should be changed.

Thats fine. Based on my initial tests I thought its not going to work with a module but it seems like this was a mistake. The module based version is way better :+1:

There's a formatting error on CI that I don't understand:

Error: rustfmt version (1.4.26) doesn't match the required version (1.4.22)

But the version required in rustfmt.toml is 1.4.11.

It seems like the CI for a merge request is not only based on the incoming branch but on the expected merge, as v1.4.22 is the version of rustfmt on the main branch.

[0ndorio]

I just checked the logs and it seems like there is still an issue with the doctest and older compiler versions even if we apply the custom rustversion attribute. The 1.38 build still tries to access the #[cfg(doctest)] attribute and fails as it was experimental at this time.

Based on some quick tests with different compiler versions it looks like:

• v1.36.0 and v1.37.0 don't complain about #[cfg(doctest)] as they just don't know about it
• starting from v1.38.0 until 1.40.0 the compiler complains as the feature is experimental

Further:

• To hide #[cfg(doctest)] for an older compiler versions it is necessary it isn't enough to put both attributes at the same element. The #[cfg(doctest)] has to be hidden inside an element tagged with #[rustversion], but currently this would results in error[E0658]: custom attributes cannot be applied to modules. I assume that was the reason I finally settled with a const variable instead of a module during the weekend.

• The release mode compilation encounters the element tagged with #[rustversion], therefore we can't introduce it as a dev-dependency (error[E0433]: failed to resolve: use of undeclared type or module `rustversion`). I tried to work around this limitation with #[cfg(test)] but it seems like this isn't possible.

[casey]

There are a few separate issues that I encountered:

1. 1.36.0 is failing due to use of an unstable feature in a dependency, it looks like this is happening in master: https://github.com/casey/bendy/runs/1421725796?check_suite_focus=true#step:8:32 I'm not sure when this started though

2. The #[rustversion] issue, which I fixed by going back to using a const, like you suggested.

3. The version of rustfmt that ships with nightly keeps getting updated, which causes formatting failures when it no longer agrees with that in rustfmt.toml. I would actually suggest removing the required_version attribute. I used to use that in a couple of projects, but ultimately found that it was annoying when it periodically caused CI breakages.

[casey]

I think failure of the object crate to compile probably was do to a change in a downstream dependency, since I checked out old versions of bendy and the compilation failure still occurred on 1.36.

[0ndorio]

Yeah the failure in object is related to an update of a downstream dependency of failure. As we are already planning to remove it I wouldn't care about it until #50 got merged.

[casey]

I opened #51 and #52 to fix the macOS failure, and to create a place to discuss no longer requiring a specific version of rustfmt. Once those issues are resolved, along with failure being removed, this should pass CI.

[thequux]

Well, CI blew up completely on this one, but that's entirely clippy issues unrelated to this PR. I'll merge this, then fix them up in a separate PR.