Re:Finding CSV files that start with a BOM using ripgrep
by burntsushi on Saturday May 29, 2021 @08:14PM
(#27329435)
You're right, I probably should have added a unit test. I added one in a branch of ongoing work. I don't always add unit tests, but in this case, it was pretty easy to. (FWIW, this is in the grep-regex crate, not the regex crate. The grep-regex crate is a glue layer between regex and ripgrep that adds in a bunch of line-oriented optimizations.)
> While repeating yourself exactly once isn't automatically a bad sign, it smells suspicious. It seems like there would be a lot of tests that ought to behave exactly the same with or without --mmap and so maybe that's a pattern worth extracting.
Quite possibly. Not a bad idea. I already do that in the tests for PCRE2. Most tests are run with both the default regex engine and with PCRE2. (There are some tests that have intended behavioral differences with mmap enabled, but those can be handled on a case-by-case basis.)
1. These seem like they're effectively integration tests. They check the entire ripgrep command line app works as intended. Is this because the bug was not where it looks like it is, in the regex crate, but elsewhere? If not, it seems like they'd be better as unit tests closer to where the bugs they're likely to detect would lie?
2. While repeating yourself exactly once isn't automatically a bad sign, it smells suspicious. It seems like there would be a lot of tests that ought to behave exactly the same with or without --mmap and so maybe that's a pattern worth extracting.