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

Inner function cannot be tested #36629

Open
kindlychung opened this issue Sep 21, 2016 · 8 comments
Open

Inner function cannot be tested #36629

kindlychung opened this issue Sep 21, 2016 · 8 comments
Labels
A-libtest Area: #[test] related C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@kindlychung
Copy link
Contributor

kindlychung commented Sep 21, 2016

I really would like to be able to this:

fn outer(x: i32) -> i32 {
    fn inner(a: i32, b: i32) -> i32 {
        a + b
    }
    #[test]
    fn test_inner() {
        assert_eq!(inner(3, 5), 8);
    }
    inner(x, 5)
}

#[test]
fn test_outer() {
    assert_eq!(outer(3), 8);
}
@Cobrand
Copy link
Contributor

Cobrand commented Sep 21, 2016

But why though ? Do you want to be able to test things inner to your function ? How about an assert_test! that would compile / trigger only when in test mode ? Just an idea.

@kindlychung
Copy link
Contributor Author

Because this is convenient for testing nested functions. Currently I'd have to move the fn outside to be able to test it separately.

@kindlychung
Copy link
Contributor Author

@Cobrand Yeah, something like this will be great:

assert!
assert_eq!
debug_assert!
debug_assert_eq!
test_assert!
test_assert_eq!

@Cobrand
Copy link
Contributor

Cobrand commented Sep 21, 2016

I'm just arguing against my own idea here, but that would require either :

  • Require to recompile everything related for "test", which I don't know which is a good idea or not
  • Compile this only in debug mode, but only run it when doing test (so it's still in the binary but there is a conditional for whether or not this is in test mode)

My code is horrible so it doesn't have that much tests, is it possible to run tests in release mode ? If that is the case, the second option is a no-no I guess.

@Aatch
Copy link
Contributor

Aatch commented Sep 22, 2016

The problem with this is that the test harness needs to access the inner functions, which it can't do because you can't name them.

@Mark-Simulacrum
Copy link
Member

I'm marking as a diagnostics issue so that we print a warning in this case at least.

@Mark-Simulacrum Mark-Simulacrum added the A-diagnostics Area: Messages for errors, warnings, and lints label May 13, 2017
@Mark-Simulacrum Mark-Simulacrum changed the title Test attribute inside an fn Inner function cannot be tested May 13, 2017
@Mark-Simulacrum Mark-Simulacrum added the A-libtest Area: #[test] related label Jun 22, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 26, 2017
bors added a commit that referenced this issue Jul 3, 2018
Add lint warning for inner function marked as `#[test]`

Fix #36629.
@eddyb
Copy link
Member

eddyb commented Jul 13, 2018

Why was the #51450 PR accepted, while the RFC is still unmerged?

I disagree with the approach taken (a new lint), we should either (rust-lang/rfcs#2471 (comment)):

@eddyb eddyb reopened this Jul 13, 2018
@estebank estebank added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed A-diagnostics Area: Messages for errors, warnings, and lints labels Oct 11, 2019
@PaulDotSH

This comment was marked as spam.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 4, 2023
…trochenkov

Make test harness lint about unnnameable tests.

Implementation of rust-lang#113734 (comment)

About the options suggested in rust-lang#36629 (comment): adding this case to unused_attribute was just more complicated. I'll try to understand a bit more what you had in mind in rust-lang/rfcs#2471 (comment)

This was just simpler to do in a standalone PR. I'll remove the corresponding changes from rust-lang#113734 later.

r? `@petrochenkov`
RalfJung pushed a commit to RalfJung/miri that referenced this issue Aug 5, 2023
Make test harness lint about unnnameable tests.

Implementation of rust-lang/rust#113734 (comment)

About the options suggested in rust-lang/rust#36629 (comment): adding this case to unused_attribute was just more complicated. I'll try to understand a bit more what you had in mind in rust-lang/rfcs#2471 (comment)

This was just simpler to do in a standalone PR. I'll remove the corresponding changes from rust-lang/rust#113734 later.

r? `@petrochenkov`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: #[test] related C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: No status
Development

No branches or pull requests

7 participants