🐥 baby steps towards stable Rust

Started working on #4 (support for stable Rust). First issue we need to solve is to get access to the harness (since we don't really want to implement it ourselves).

There is https://crates.io/crates/libtest crate, which is recent version of Rust internal test harness, extracted as a crate. However, it only compiles on nightly, so it won't help us here.

There is also https://crates.io/crates/rustc-test, but it is 2 years old. I haven't checked its features, but might not support some of the desired functionality (like, JSON output in tests? colored output?).

So, the third option (which I'm using here) is to use `test` crate from the Rust itself and also set `RUSTC_BOOTSTRAP=1` for our crate so we can access it on stable channel. Not great, but works for now.

Second issue is to get access to the tests. On nightly, we use `#[test_case]` to hijack Rust tests registration so we can get access to them in nightly.

Cannot do that on stable. What would help here is something along the lines of https://internals.rust-lang.org/t/idea-global-static-variables-extendable-at-compile-time/9879 or https://internals.rust-lang.org/t/pre-rfc-add-language-support-for-global-constructor-functions. Don't have that, so we use https://crates.io/crates/ctor crate to build our own registry of tests, similar to https://crates.io/crates/inventory.

The caveat here is potentially hitting dtolnay/inventory#7 issue which would manifest itself as test being silently ignored. Not great, but let's see how bad it will be.

Third piece of the puzzle is to intercept execution of tests. This is done by asking users to use `harness = false` in their `Cargo.toml`, in which case we take full control of test execution.

Finally, the last challenge is that with `harness = false`, we don't have a good way to intercept "standard" tests (`#[test]`): https://users.rust-lang.org/t/capturing-test-when-harness-false-in-cargo-toml/28115

So, the plan here is to provide `#[datatest::test]` attribute that will behave similar to built-in `#[test]` attribute, but will use our own registry for tests. No need to support `#[bench]` as it is not supported on stable channel anyway.

The caveat in this case is that if you use built-in `#[test]`, your test will be silently ignored. Not great, not sure what to do about it.

Proper solution, of course, would be driving RFC for custom test frameworks: rust-lang/rust#50297 😅

Partially fixes #4 (still missing support for standard tests and also documentation).

Author: Ivan Dubrov

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "datatest"
-version = "0.3.5"
+version = "0.4.0"
 authors = ["Ivan Dubrov <[email protected]>"]
 edition = "2018"
 repository = "https://github.com/commure/datatest"
@@ -10,13 +10,21 @@ description = """
 Data-driven tests in Rust
 """
 
+[[test]]
+name = "datatest_stable"
+harness = false
+
+[build-dependencies]
+version_check = "0.9.1"
+
 [dependencies]
-datatest-derive = { path = "datatest-derive", version = "=0.3.5" }
+datatest-derive = { path = "datatest-derive", version = "=0.4.0" }
 regex = "1.0.0"
 walkdir = "2.1.4"
 serde = "1.0.84"
 serde_yaml = "0.8.7"
 yaml-rust = "0.4.2"
+ctor = "0.1.10"
 
 [dev-dependencies]
 serde = { version = "1.0.84", features = ["derive"] }

build.rs

@@ -0,0 +1,11 @@
+use version_check::Channel;
+
+fn main() {
+    let is_nightly = Channel::read().map_or(false, |ch| ch.is_nightly());
+    if is_nightly {
+        println!("cargo:rustc-cfg=feature=\"nightly\"");
+    } else {
+        println!("cargo:rustc-cfg=feature=\"stable\"");
+    }
+    println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1");
+}

datatest-derive/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "datatest-derive"
-version = "0.3.5"
+version = "0.4.0"
 authors = ["Ivan Dubrov <[email protected]>"]
 edition = "2018"
 repository = "https://github.com/commure/datatest"

datatest-derive/src/lib.rs

@@ -2,19 +2,14 @@
 #![deny(unused_must_use)]
 extern crate proc_macro;
 
-#[macro_use]
-extern crate syn;
-#[macro_use]
-extern crate quote;
-extern crate proc_macro2;
-
 use proc_macro2::{Span, TokenStream};
+use quote::quote;
 use std::collections::HashMap;
 use syn::parse::{Parse, ParseStream, Result as ParseResult};
 use syn::punctuated::Punctuated;
 use syn::spanned::Spanned;
 use syn::token::Comma;
-use syn::{ArgCaptured, FnArg, Ident, ItemFn, Pat};
+use syn::{braced, parse_macro_input, ArgCaptured, FnArg, Ident, ItemFn, Pat};
 
 type Error = syn::parse::Error;
 
@@ -90,6 +85,29 @@ impl Parse for FilesTestArgs {
     }
 }
 
+enum Channel {
+    Stable,
+    Nightly,
+}
+
+/// Wrapper that turns on behavior that works on stable Rust.
+#[proc_macro_attribute]
+pub fn files_stable(
+    args: proc_macro::TokenStream,
+    func: proc_macro::TokenStream,
+) -> proc_macro::TokenStream {
+    files_internal(args, func, Channel::Stable)
+}
+
+/// Wrapper that turns on behavior that works only on nightly Rust.
+#[proc_macro_attribute]
+pub fn files_nightly(
+    args: proc_macro::TokenStream,
+    func: proc_macro::TokenStream,
+) -> proc_macro::TokenStream {
+    files_internal(args, func, Channel::Nightly)
+}
+
 /// Proc macro handling `#[files(...)]` syntax. This attribute defines rules for deriving
 /// test function arguments from file paths. There are two types of rules:
 /// 1. Pattern rule, `<arg_name> in "<regexp>"`
@@ -131,11 +149,10 @@ impl Parse for FilesTestArgs {
 /// I could have made this proc macro to handle these cases explicitly and generate a different
 /// code, but I decided to not add a complexity of type analysis to the proc macro and use traits
 /// instead. See `datatest::TakeArg` and `datatest::DeriveArg` to see how this mechanism works.
-#[proc_macro_attribute]
-#[allow(clippy::needless_pass_by_value)]
-pub fn files(
+fn files_internal(
     args: proc_macro::TokenStream,
     func: proc_macro::TokenStream,
+    channel: Channel,
 ) -> proc_macro::TokenStream {
     let mut func_item = parse_macro_input!(func as ItemFn);
     let args: FilesTestArgs = parse_macro_input!(args as FilesTestArgs);
@@ -195,7 +212,7 @@ pub fn files(
 
                     params.push(arg.value.value());
                     invoke_args.push(quote! {
-                        ::datatest::TakeArg::take(&mut <#ty as ::datatest::DeriveArg>::derive(&paths_arg[#idx]))
+                        ::datatest::__internal::TakeArg::take(&mut <#ty as ::datatest::__internal::DeriveArg>::derive(&paths_arg[#idx]))
                     })
                 } else {
                     return Error::new(pat_ident.span(), "mapping is not defined for the argument")
@@ -231,31 +248,34 @@ pub fn files(
     let orig_func_name = &func_item.ident;
 
     let (kind, bencher_param) = if info.bench {
-        (quote!(BenchFn), quote!(bencher: &mut ::datatest::Bencher,))
+        (
+            quote!(BenchFn),
+            quote!(bencher: &mut ::datatest::__internal::Bencher,),
+        )
     } else {
         (quote!(TestFn), quote!())
     };
 
-    // Adding `#[allow(unused_attributes)]` to `#orig_func` to allow `#[ignore]` attribute
+    let registration = test_registration(channel, &desc_ident);
     let output = quote! {
-        #[test_case]
+        #registration
         #[automatically_derived]
         #[allow(non_upper_case_globals)]
-        static #desc_ident: ::datatest::FilesTestDesc = ::datatest::FilesTestDesc {
+        static #desc_ident: ::datatest::__internal::FilesTestDesc = ::datatest::__internal::FilesTestDesc {
             name: concat!(module_path!(), "::", #func_name_str),
             ignore: #ignore,
             root: #root,
             params: &[#(#params),*],
             pattern: #pattern_idx,
             ignorefn: #ignore_func_ref,
-            testfn: ::datatest::FilesTestFn::#kind(#trampoline_func_ident),
+            testfn: ::datatest::__internal::FilesTestFn::#kind(#trampoline_func_ident),
         };
 
         #[automatically_derived]
         #[allow(non_snake_case)]
         fn #trampoline_func_ident(#bencher_param paths_arg: &[::std::path::PathBuf]) {
             let result = #orig_func_name(#(#invoke_args),*);
-            datatest::assert_test_result(result);
+            ::datatest::__internal::assert_test_result(result);
         }
 
         #func_item
@@ -323,11 +343,28 @@ impl Parse for DataTestArgs {
     }
 }
 
+/// Wrapper that turns on behavior that works on stable Rust.
 #[proc_macro_attribute]
-#[allow(clippy::needless_pass_by_value)]
-pub fn data(
+pub fn data_stable(
     args: proc_macro::TokenStream,
     func: proc_macro::TokenStream,
+) -> proc_macro::TokenStream {
+    data_internal(args, func, Channel::Stable)
+}
+
+/// Wrapper that turns on behavior that works only on nightly Rust.
+#[proc_macro_attribute]
+pub fn data_nightly(
+    args: proc_macro::TokenStream,
+    func: proc_macro::TokenStream,
+) -> proc_macro::TokenStream {
+    data_internal(args, func, Channel::Nightly)
+}
+
+fn data_internal(
+    args: proc_macro::TokenStream,
+    func: proc_macro::TokenStream,
+    channel: Channel,
 ) -> proc_macro::TokenStream {
     let mut func_item = parse_macro_input!(func as ItemFn);
     let cases: DataTestArgs = parse_macro_input!(args as DataTestArgs);
@@ -376,23 +413,24 @@ pub fn data(
 
     let (case_ctor, bencher_param, bencher_arg) = if info.bench {
         (
-            quote!(::datatest::DataTestFn::BenchFn(Box::new(::datatest::DataBenchFn(#trampoline_func_ident, case)))),
-            quote!(bencher: &mut ::datatest::Bencher,),
+            quote!(::datatest::__internal::DataTestFn::BenchFn(Box::new(::datatest::__internal::DataBenchFn(#trampoline_func_ident, case)))),
+            quote!(bencher: &mut ::datatest::__internal::Bencher,),
             quote!(bencher,),
         )
     } else {
         (
-            quote!(::datatest::DataTestFn::TestFn(Box::new(move || #trampoline_func_ident(case)))),
+            quote!(::datatest::__internal::DataTestFn::TestFn(Box::new(move || #trampoline_func_ident(case)))),
             quote!(),
             quote!(),
         )
     };
 
+    let registration = test_registration(channel, &desc_ident);
     let output = quote! {
-        #[test_case]
+        #registration
         #[automatically_derived]
         #[allow(non_upper_case_globals)]
-        static #desc_ident: ::datatest::DataTestDesc = ::datatest::DataTestDesc {
+        static #desc_ident: ::datatest::__internal::DataTestDesc = ::datatest::__internal::DataTestDesc {
             name: concat!(module_path!(), "::", #func_name_str),
             ignore: #ignore,
             describefn: #describe_func_ident,
@@ -402,12 +440,12 @@ pub fn data(
         #[allow(non_snake_case)]
         fn #trampoline_func_ident(#bencher_param arg: #ty) {
             let result = #orig_func_ident(#bencher_arg #ref_token arg);
-            datatest::assert_test_result(result);
+            ::datatest::__internal::assert_test_result(result);
         }
 
         #[automatically_derived]
         #[allow(non_snake_case)]
-        fn #describe_func_ident() -> Vec<::datatest::DataTestCaseDesc<::datatest::DataTestFn>> {
+        fn #describe_func_ident() -> Vec<::datatest::DataTestCaseDesc<::datatest::__internal::DataTestFn>> {
             let result = #cases
                 .into_iter()
                 .map(|input| {
@@ -427,3 +465,29 @@ pub fn data(
     };
     output.into()
 }
+
+fn test_registration(channel: Channel, desc_ident: &syn::Ident) -> TokenStream {
+    match channel {
+        // On nightly, we rely on `custom_test_frameworks` feature
+        Channel::Nightly => quote!(#[test_case]),
+        // On stable, we use `ctor` crate to build a registry of all our tests
+        Channel::Stable => {
+            let registration_fn =
+                syn::Ident::new(&format!("{}__REGISTRATION", desc_ident), desc_ident.span());
+            let tokens = quote! {
+                #[allow(non_snake_case)]
+                #[datatest::__internal::ctor]
+                fn #registration_fn() {
+                    use ::datatest::__internal::RegistrationNode;
+                    static mut REGISTRATION: RegistrationNode = RegistrationNode {
+                        descriptor: &#desc_ident,
+                        next: None,
+                    };
+                    // This runs only once during initialization, so should be safe
+                    ::datatest::__internal::register(unsafe { &mut REGISTRATION });
+                }
+            };
+            tokens
+        }
+    }
+}

src/lib.rs

@@ -115,17 +115,28 @@ mod data;
 mod files;
 mod runner;
 
+/// Internal re-exports for the procedural macro to use.
 #[doc(hidden)]
-pub use crate::data::{DataBenchFn, DataTestDesc, DataTestFn};
-#[doc(hidden)]
-pub use crate::files::{DeriveArg, FilesTestDesc, FilesTestFn, TakeArg};
-#[doc(hidden)]
-pub use crate::runner::{assert_test_result, runner};
+pub mod __internal {
+    pub use crate::data::{DataBenchFn, DataTestDesc, DataTestFn};
+    pub use crate::files::{DeriveArg, FilesTestDesc, FilesTestFn, TakeArg};
+    pub use crate::runner::assert_test_result;
+    pub use crate::test::Bencher;
+    pub use ctor::ctor;
+
+    // To maintain registry on stable channel
+    pub use crate::runner::{register, RegistrationNode};
+}
+
+pub use crate::runner::runner;
+
 #[doc(hidden)]
-pub use crate::test::Bencher;
+#[cfg(feature = "stable")]
+pub use datatest_derive::{data_stable as data, files_stable as files};
 
 #[doc(hidden)]
-pub use datatest_derive::{data, files};
+#[cfg(feature = "nightly")]
+pub use datatest_derive::{data_nightly as data, files_nightly as files};
 
 /// Experimental functionality.
 #[doc(hidden)]
@@ -135,6 +146,23 @@ use std::fs::File;
 use std::io::{BufReader, Read};
 use std::path::Path;
 
+/// `datatest` test harness entry point. Should be declared in the test module, like in the
+/// following snippet:
+/// ```rust,norun
+/// datatest::harness!();
+/// ```
+///
+/// Also, `harness` should be set to `false` for that test module in `Cargo.toml` (see [Configuring a target](https://doc.rust-lang.org/cargo/reference/manifest.html#configuring-a-target)).
+#[macro_export]
+macro_rules! harness {
+    () => {
+        #[cfg(test)]
+        fn main() {
+            ::datatest::runner(&[]);
+        }
+    };
+}
+
 /// Helper function used internally.
 fn read_to_string(path: &Path) -> String {
     let mut input = String::new();