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

Bad custom test names should not fail silently #72

Open
GabrielSimonetto opened this issue Oct 31, 2021 · 4 comments
Open

Bad custom test names should not fail silently #72

GabrielSimonetto opened this issue Oct 31, 2021 · 4 comments
Labels
enhancement New feature or request spike Need to research topic more to come up with a solution
Projects

Comments

@GabrielSimonetto
Copy link

GabrielSimonetto commented Oct 31, 2021

I will be borrowing the example from #19, since it has some relation to how I found this error on my own code.

So, we know from that issue that the minus will be abstracted from the automatic name creation process, BUT, we have a mechanism for custom names, and that could help the developer to understand that the problem with his code is that he is lacking a valid test name:

#[test_case(15, 15; "Abs_15_15")]
#[test_case(-15, 15; "Abs_-15_-15")]
fn test_crazy_stuff(value: i32, expected: i32) {
    assert_eq!(value.abs(), expected)
}

And this will raise a familiar error, but the interesting part is that it tried to use my custom name, since it begins with abs, and not test_crazy_stuff:

error[E0428]: the name `abs_15_15` is defined multiple times
   --> src/lib.rs:162:5
    |
162 |     #[test_case(15, 15; "Abs_15_15")]
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |     |
    |     `abs_15_15` redefined here
    |     previous definition of the value `abs_15_15` here

This leads me to believe that we have a process that tries to make the name valid, since it erased the hyphen from my custom test, applied, and then we had a name collision.

I haven't tried to analyze the code to understand how the hyphen is erased in both cases, but, i believe that if a developer instantiates a custom name, it shouldn't be safeguarded, if the name is invalid it should break catastrophically warning the developer of what the problem is.

@frondeus
Copy link
Owner

frondeus commented Nov 1, 2021

That's an exciting idea. However, the intention was to provide a human-readable test description, not necessarily a compiler-readable one. For example:

    #[test_case(4,  2  ; "when operands are swapped")]
    #[test_case(-2, -4 ; "when both operands are negative")]
    #[test_case(2,  4  ; "when both operands are positive")]

As you can see, in this example, we have to sanitize input.

Anyway, you have a valid point. @luke-biel - perhaps we should introduce some kind of feature flag or syntax (like different quotes used) to enable "strict mode"?

@GabrielSimonetto
Copy link
Author

the intention was to provide a human-readable test description, not necessarily a compiler-readable one.

Hmm, yes, that makes sense

@luke-biel
Copy link
Collaborator

perhaps we should introduce some kind of feature flag or syntax (like different quotes used) to enable "strict mode"?
@frondeus I'd see extra arg to test case rather than changing syntax of the description.

#[test_case(4, 2 ; "when operands are swapped", fn_name = my_very_custom_name)]

I'm just not sure if this doesn't introduce bloat within test-case - how many people are gonna use it?

@luke-biel luke-biel mentioned this issue Dec 18, 2021
7 tasks
@luke-biel luke-biel added enhancement New feature or request spike Need to research topic more to come up with a solution labels Jan 28, 2022
@luke-biel luke-biel added this to To do in 2.1.0 Apr 22, 2022
@luke-biel luke-biel moved this from To do to Backlog in 2.1.0 May 12, 2022
@AugustoFKL
Copy link
Contributor

IMHO, a project using test_case should almost always add a name to the test, for good practices. But I think refining the opposite is still something good.

The solution provided on #19 would solve this problem, but the question is: what defined a "bad test name"? Anything besides alphanumeric, spaces, and underscores?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request spike Need to research topic more to come up with a solution
Projects
No open projects
2.1.0
Backlog
Development

No branches or pull requests

4 participants