Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test module name wrong detection regression #79

Open
Dushistov opened this issue Jan 2, 2019 · 7 comments
Open

test module name wrong detection regression #79

Dushistov opened this issue Jan 2, 2019 · 7 comments

Comments

@Dushistov
Copy link
Contributor

Dushistov commented Jan 2, 2019

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 8a79f4e..5462994
(cd ~/.emacs.d/elpa/cargo-20190101.2043 && patch -p1 -R)

and all starts work again as expected

cc @ralexstokes @kwrooijen

@kwrooijen
Copy link
Owner

I'll have a look once I get home

@ralexstokes
Copy link
Contributor

@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... (rust-lang/rust#37873)

@ralexstokes
Copy link
Contributor

ralexstokes commented Jan 2, 2019

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)]?

@kwrooijen
Copy link
Owner

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?

@ralexstokes
Copy link
Contributor

ralexstokes commented Jan 2, 2019

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.

@Dushistov
Copy link
Contributor Author

@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
Copy link
Contributor Author

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() {
...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants