nextest-rs / datatest-stable

Data-driven tests on stable Rust
https://docs.rs/datatest-stable
Other
36 stars 6 forks source link

The :path designator is too restrictive in the $name parameter #4

Closed badicsalex closed 2 years ago

badicsalex commented 2 years ago

Hi,

I have been using this harness with Great Success (tm), apart from a minor inconvenience.

Background:

To reduce boilerplate with tests separated into modules, I made the following wrapper:

macro_rules! generate_harness{
    ($($test:ident),*) => {
        datatest_stable::harness!(
            $(
                $test::run_test,
                $test::test_dir(),
                $test::FILE_PATTERN,
            )*
        );
    }
}

generate_harness!(test_pdf_parser, test_structure_parser, test_reference_parsing);

Problem

Unfortunately if I modify the wrapper to use paths:

macro_rules! generate_harness{
    ($($test:path),*) => {
        datatest_stable::harness!(
            $(
                $test::run_test,
                $test::test_dir(),
                $test::FILE_PATTERN,
            )*
        );
    }
}

generate_harness!(pdf::test_pdf_parser, structure::test_structure_parser, grammar::test_reference_parsing);

Cause

It suddenly doesn't work, because rustc cannot properly comprehend $test::run_test if $test a path and not an ident.

This is due to https://github.com/rust-lang/rust/issues/48067.

Solution?

Apparently, one possible workaround is using $Va : ident $( :: $Vb : ident )* instead of path, but I have not tested this.

Loosening $name to expr would also solve the problem I think.

I'd also be happy with any other idea. What I currently do is just import everything into the crate namespace, but that's going to be problematic sooner or later.

sunshowers commented 2 years ago

Thanks for the report! Could you try the use as workaround listed in the issue and see if that works? If so, I'd love a PR. Thanks!

badicsalex commented 2 years ago

I've been playing around with it since the report, and this worked without touching the harness macro itself:

macro_rules! generate_harness{
    ($($first:ident$(::$rest:ident)*),*) => {
        datatest_stable::harness!(
            $(
                $first$(::$rest)*::run_test,
                $first$(::$rest)*::test_dir(),
                $first$(::$rest)*::FILE_PATTERN,
            )*
        );
    }
}

generate_harness!(
    fast::test_pdf_parser,
    fast::structure::test_structure_parser,
    slow::grammar::test_reference_parsing
);

It would be nice to be able to put an expr in the $name field too though, because it was very convenient to use a function returning a string in the directory field, and I'd imagine a function returning a function or similar would be a valid use-case for the test function too.

But since the original problem could be solved without modifying the datatest library, I guess the issue can be closed.