cargo.el icon indicating copy to clipboard operation
cargo.el copied to clipboard

test module name wrong detection regression

Open Dushistov opened this issue 5 years ago • 7 comments

Looks like #76 breaks test running thing for me.

I have

My code looks like this:

mod tests {
    #[test]
    fn test_double_map_err() {
        do_parse(
            "double_map_err",
            r#"
mod swig_foreign_types_map {}
mod swig_foreign_types_map {}
"#,
            64,
            HashMap::new(),
        )
        .unwrap_err();
    }

    #[test]
   fn another_test() {
   }
}

And after #76 cargo.el tries to run swig_foreign_types_map::another_test instead of tests::another_test

I revert git diff 8a79f4edb31a1d821d04f4c8f10d31f4297eee2d..5462994393b01b85a76caacf9277bff431211ae4 (cd ~/.emacs.d/elpa/cargo-20190101.2043 && patch -p1 -R)

and all starts work again as expected

cc @ralexstokes @kwrooijen

Dushistov avatar Jan 02 '19 12:01 Dushistov

I'll have a look once I get home

kwrooijen avatar Jan 02 '19 13:01 kwrooijen

@Dushistov @kwrooijen the way it was working is that emacs would regexp-search for the first prior instance that matched the test-regexp.

i think what is going on is that the regexp before this patch was ignoring the mod decls you have in the raw string because they have _ in them which was not being recognized as a valid \w char. Given that _ is a valid char in Rust identifiers I would suggest we stick with the new regexp.

@Dushistov i haven't tried it but i suspect if you s/swig_foreign_types_map/somethingwithoutunderbar/ then you'll see the behavior you expect.

I would say the way to get around this particular instance involves tracking if we are inside a string context and ignoring those entries. A better way to fix this would be to work over the rustc AST and go up one module node -- however it looks like we can only dump the AST on the nightly compiler... (https://github.com/rust-lang/rust/issues/37873)

ralexstokes avatar Jan 02 '19 15:01 ralexstokes

just ran into another behavior i want in this mode for cargo-process-current-file-tests which made me realize we can try something else that may fix all of these issues and not involve string tracking or AST munging...

working on top of #78, if you run cargo-process-current-file-tests inside a test module, it will run just the tests in that file (assuming you don't have other mod decls like @Dushistov pointed out).

if you are outside of that module, but still in that same buffer all tests will run. so a simple fix here is to move point to the very beginning of the file and then find the first mod decl. this could be extended to: find the closest top-level mod decl that is nearest the point when we begin this command.

another option is to just search for #[cfg(test)] although it seems like that only is used (and is still optional) in "unit test" style tests in line with the source. I haven't tried using this function in /test integration style tests so unclear how it behaves. any thoughts on trying to find #[cfg(test)]?

ralexstokes avatar Jan 02 '19 18:01 ralexstokes

if you are outside of that module, but still in that same buffer all tests will run.

You mean outside of the mod declaration?

If #[cfg(test)] is optional then that doesn't sound like a reliable option, unless I'm misunderstanding?

kwrooijen avatar Jan 02 '19 22:01 kwrooijen

You mean outside of the mod declaration?

yep -- like this, point is where the | is:

fn foo() {}

mod tests {
  #[test]
|  fn it_works() {}
}

=> function will run cargo test whatever::tests

fn foo() {}
|
mod tests {
  #[test]  
  fn it_works() {}
}

=> function will run cargo test

If #[cfg(test)] is optional then that doesn't sound like a reliable option, unless I'm misunderstanding?

it is optional but afaik a strong convention. it won't be 100% reliable, so then i would suggest we start from the top of the file and find the closest mod decl to the current point. we can again rely on strong convention and find a mod decl that begins at the start of a line (a la rustfmt). or we would have to go back to skipping over strings (and this opens a can of worms for other types of special contexts we would have to track inside the minor mode). one thing i found earlier that could be useful is cargo test -- --list. so we could build the stream of mod decls in the current buffer ordered by distance and then filter for those in that list given by cargo.

ralexstokes avatar Jan 02 '19 23:01 ralexstokes

@ralexstokes @kwrooijen

If mod name search is not reliable, may be add option to disable it?

And run cargo test without mod name, for example cargo test test_unpack_first_associated_type instead of cargo test typemap::parse::tests::test_unpack_first_associated_type

In my code I have no test function with the same name, and for such rare case when somebody has, for me running two tests is better than running 0 tests.

Dushistov avatar Jan 03 '19 03:01 Dushistov

One more corner test-case is usage of syn / quote, so ignoring of string literals is not enough, also it would be good to ignore macro invocations.

#[test]
fn test1() {
  let item_mod: syn::ItemMod = parse_quote! {
         mod aaa {
         }
  };
...
}

#[test]
fn test2() {
...
}

Dushistov avatar Jan 03 '19 03:01 Dushistov