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

Regression test #210 fails on macOS High Sierra #559

Closed
jhenahan opened this issue Jul 17, 2017 · 2 comments
Closed

Regression test #210 fails on macOS High Sierra #559

jhenahan opened this issue Jul 17, 2017 · 2 comments
Labels
bug A bug.

Comments

@jhenahan
Copy link

I'm not positive as to why, but I have a feeling it may be related to normalization-insensitivity in APFS. If I understand the APFS docs and the test case correctly, this may actually be an issue in std::ffi, and if that's the case then I'll report this up the chain.

Stack trace:

failures:

---- regression_210 stdout ----
	thread 'regression_210' panicked at '~/src/ripgrep/target/debug/deps/ripgrep-tests/regression_210/93/foo�bar: Error { repr: Os { code: 92, message: "Illegal byte sequence" } }', tests/workdir.rs:285
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: integration::workdir::nice_err
   7: integration::workdir::WorkDir::create_bytes
   8: integration::workdir::WorkDir::create
   9: integration::regression_210
  10: <F as test::FnBox<T>>::call_box
  11: __rust_maybe_catch_panic
  12: std::panicking::try::do_call
  13: __rust_maybe_catch_panic
  14: <F as alloc::boxed::FnBox<A>>::call_box
  15: std::sys::imp::thread::Thread::new::thread_start
  16: _pthread_body
  17: _pthread_start


failures:
    regression_210

test result: FAILED. 136 passed; 1 failed; 0 ignored; 0 measured
@BurntSushi
Copy link
Owner

BurntSushi commented Jul 17, 2017

This is the test in question:

// See: https://github.com/BurntSushi/ripgrep/issues/210
#[cfg(unix)]
#[test]
fn regression_210() {
    use std::ffi::OsStr;
    use std::os::unix::ffi::OsStrExt;

    let badutf8 = OsStr::from_bytes(&b"foo\xffbar"[..]);

    let wd = WorkDir::new("regression_210");
    let mut cmd = wd.command();
    wd.create(badutf8, "test");
    cmd.arg("-H").arg("test").arg(badutf8);

    let out = wd.output(&mut cmd);
    assert_eq!(out.stdout, b"foo\xffbar:test\n".to_vec());
}

This test is specifically testing a case where there is an invalid UTF-8 byte in a file path (which is a valid thing to do) and that ripgrep still happily searches said file.

The APFS docs link says this:

APFS accepts only valid UTF-8 encoded filenames for creation

Which means this test probably doesn't make sense on a file system that specifically rejects invalid UTF-8. This also means that I don't think there is any bug in std::ffi, and that according to the APFS docs, this test is failing in exactly the way I'd expect.

I think the proper work-around here is to modify the test to pass if it is unable to create a file with an invalid UTF-8 name.

@BurntSushi BurntSushi added the bug A bug. label Jul 17, 2017
@DanielJoyce
Copy link

Or you can add a #cfg[] param to exclude macosx so the test isn't run at all on that platform

Something like

#[cfg(all(unix,not(target_os="macos")))]

sebnow added a commit to sebnow/ripgrep that referenced this issue Oct 16, 2017
APFS does not support creating filenames with invalid UTF-8 byte codes,
thus this test doesn't make sense. Skip it on file systems where this
shouldn't be possible.

Fixes BurntSushi#559
BurntSushi pushed a commit that referenced this issue Oct 21, 2017
APFS does not support creating filenames with invalid UTF-8 byte codes,
thus this test doesn't make sense. Skip it on file systems where this
shouldn't be possible.

Fixes #559
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug.
Projects
None yet
Development

No branches or pull requests

3 participants