Repository navigation
Say what glob! searched when it fails - #945
Conversation
When `glob!` matched no files it panicked with only "the glob! macro did not match any files.", so it was hard to tell where it had looked. The message now shows the pattern and the resolved base directory, notes when that directory does not exist, and says the pattern is relative to the base directory. An invalid pattern and an error while walking the files panicked on an `unwrap` with the error's debug output. Both now give a short message that names the pattern or the directory. A missing base directory panicked while walking, after `glob!` had pushed its entry on the glob stack. The entry was never removed, so later snapshot failures in the same test binary were only printed and those tests passed. The checks and the walk now run before the push. Added tests in `test_glob.rs`, a unit test for the glob stack, and a functional test that snapshots the new messages. Fixes mitsuhiko#398
2183c60 to
fe9e562
Compare
|
Rebased onto master to resolve the CHANGELOG conflict with #946, no code changes. @max-sixty could you take a look when you have time? |
| fn panic_no_match(base: &Path, pattern: &str) -> ! { | ||
| let mut base_dir = base.display().to_string(); | ||
| if !base.exists() { | ||
| base_dir.push_str(" (does not exist)"); |
There was a problem hiding this comment.
presumably this isn't always right though? how about if it doesn't have permission?
There was a problem hiding this comment.
Right, exists() returns false for any error. It now uses try_exists, so only a base that is not found is reported as missing. Any other error, like a permission error, goes on to the walk, which panics with the io error: the glob! macro failed while walking <base>: IO error for operation on <base>: <error>. A permission error is hard to set up the same way on Linux, macOS and Windows, so the new test uses a NUL byte in the base path, which fails with an error other than not found on all three.
The allow_empty_glob early return above still uses exists(), so there a base that can't be checked is still skipped. I left it alone, since switching it would turn that silent skip into a panic. Happy to change it too if you prefer.
| base_dir.push_str(" (does not exist)"); | ||
| } | ||
| panic!( | ||
| "the glob! macro did not match any files.\n\ |
There was a problem hiding this comment.
nit but why these line endings?
There was a problem hiding this comment.
The idea was to give the pattern and the base directory their own lines, but one line fits the other glob! panics better. It's one line now:
the glob! macro did not match any files. It searched for 'src/data/*.txt' in /path/to/project/src. The pattern is relative to the base directory, which defaults to the directory of the file calling glob!.
There was a problem hiding this comment.
but what's the benefit from the line ending?
There was a problem hiding this comment.
None really. It only moved the second sentence onto its own line in the panic output. bf5447c drops it, so both no-match messages are now single lines like the other glob! panics.
`Path::exists` returns false for any error, so a base directory that could not be checked, for example because of a permission error, was reported as not existing. `glob!` now uses `try_exists`, so only a base that is not found is reported that way. Other errors are reported by the walk, with the io error. The no match message is now one line, like the other `glob!` panics.
glob!now says what it searched when it fails.Before:
After:
A base directory that is not found gives
the glob! macro did not match any files. It searched for '*.txt' in /path/to/project/src/missing, which does not exist.Before, it panicked with the walkdir error's debug output. Other errors on the base directory, such as a permission error, are reported by the walk with the io error. An invalid pattern now panics withthe glob! macro got an invalid pattern 'data/[': unclosed character class; missing ']'instead of anunwrapon the globset error, and an error while walking names the directory and the failing path.A missing base directory or an error while walking also left an entry on the glob stack, because those panics came after the push. After that, later snapshot failures in the same test binary were only printed and their tests passed. An invalid pattern panicked before the push and no match after the pop, so those two did not leak. The checks and the walk now run before the push, and a unit test checks that the stack is empty after the failing cases.
The no match message still starts with the old text, so
should_panic(expected = "the glob! macro did not match any files.")still matches.cargo-insta/tests/functional/glob_errors.rssnapshots the messages for the three cases.#623 was fixed by #743, which added the message for
../patterns, so it can be closed.Fixes #398.