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

Include the kind of test while matching filters #30

Merged
merged 3 commits into from
Jan 14, 2024

Conversation

sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Mar 22, 2023

Consider a trial with name "my_test" and kind "foo". The text that gets printed out with --list --format terse is:

[foo] my_test: test

With libtest, an invariant is that the part of the string before ": test" can be passed into "--exact" to run that test. However, that invariant is currently not upheld by libtest-mimic: it only matches against the name of the test and not the kind. This breaks nextest's ability to run these tests: nextest-rs/nextest#836

Make it so that libtest-mimic also matches against the kind of test, not just its name.

I also took the liberty of running rustfmt (or really, my editor automatically ran it when I went in to edit the file.)

tests/mixed_bag.rs Outdated Show resolved Hide resolved
Copy link
Owner

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Seems like a reasonable idea to also match the kind.

I left one comment that still worries me. And please remove the rustfmt changes so that this PR only contains the changes for the behavior change. Plus the copy&paste comment that the other reviewer found.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Consider a trial with name "my_test" and kind "foo". The text that gets
printed out with `--list --format terse` is:

```
[foo] my_test: test
```

With libtest, an invariant is that the part of the string before ":
test" can be passed into "--exact" to run that test. However, that
invariant is currently not upheld by libtest-mimic: it only matches
against the name of the test and not the kind.

Make it so that libtest-mimic also matches against the kind of test, not
just its name.

I also took the liberty of running rustfmt (or really, my editor
automatically ran it when I went in to edit the file.)
@sunshowers
Copy link
Contributor Author

And please remove the rustfmt changes so that this PR only contains the changes for the behavior change

Would you be open at all to formatting the code via rustfmt, maybe as a separate change? To be honest this is a huge discouragement for me to contribute (and why I didn't get around to this for many months), because my editor is set up to autoformat on save, and rustfmt is used by every single one of the ~100 Rust projects I've ever contributed to.

Copy link
Owner

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Also for writing tests, that's always very helpful.

@LukasKalbertodt LukasKalbertodt merged commit 3ac6132 into LukasKalbertodt:master Jan 14, 2024
2 checks passed
@LukasKalbertodt
Copy link
Owner

Would you be open at all to formatting the code via rustfmt, maybe as a separate change? To be honest this is a huge discouragement for me to contribute (and why I didn't get around to this for many months), because my editor is set up to autoformat on save, and rustfmt is used by every single one of the ~100 Rust projects I've ever contributed to.

Unfortunately, not at this time, no. I just checked all the changes made by cargo fmt again. And while most are totally fine, I disagree with some. My main concern I raised in an issue already (and quite a few people seem to agree), but changing the default was rejected and the configuration is still not stable. Another thing is that rustfmt removes trailing commas in attributes, which makes no sense to me. And it doesn't seem like there is a configuration option for this at all. I would love to use rustfmt, but there are some blockers for me. Of course, once a project grows and gets more contributors, I might change my opinion for that particular project.

And FWIW, my editor is also set up to do certain things automatically on save (removing trailing whitespace, ...). But if I contribute to some project that does not adhere to this basic code hygiene, I will disable it temporarily. Or more likely, just don't stage those changes before a commit. Personally I think that's no big deal.

That's just some explanation why things are the way they are.

@LukasKalbertodt
Copy link
Owner

LukasKalbertodt commented Jan 14, 2024

Released as v0.7.0

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

Successfully merging this pull request may close these issues.

3 participants