Bat: assets::tests::syntax_detection_invalid_utf8 failed with 0.15.1

[lilyball]

I just tried compiling bat 0.15.1 via Nix on macOS and it failed during the tests phase:

---- assets::tests::syntax_detection_invalid_utf8 stdout ----
thread 'assets::tests::syntax_detection_invalid_utf8' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 92, kind: Other, message: "Illegal byte sequence" }', src/assets.rs:287:37
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    assets::tests::syntax_detection_invalid_utf8

Also of note, the color escapes used by the test runner all stuck a visible (B after each colored string. Which is to say, each "ok" was "ok(B" and the "FAILED" was "FAILED(B". Colors printed by rustc itself were fine. I don't know if this is meaningful or just weird though.

[lilyball]

The test failure also occurs when running cargo test on a manual checkout outside of Nix. The (B oddity doesn't so I guess that's unrelated.

[lilyball]

I suspect this is a test that simply always fails on macOS. It appears to be attempting to create a filename with invalid utf8, but macOS doesn't allow this. This test will need to be disabled for macOS.

[lilyball]

The following diff fixes cargo test:

diff --git a/src/assets.rs b/src/assets.rs
index 4f8556f..222abc2 100644
--- a/src/assets.rs
+++ b/src/assets.rs
@@ -336,7 +336,7 @@ mod tests {
         assert_eq!(test.syntax_for_file("Makefile"), "Makefile");
     }

-    #[cfg(unix)]
+    #[cfg(all(unix,not(target_os = "macos")))]
     #[test]
     fn syntax_detection_invalid_utf8() {
         use std::os::unix::ffi::OsStrExt;

However it does introduce a warning

warning: method is never used: `syntax_for_file_os`
   --> src/assets.rs:301:9
    |
301 |         fn syntax_for_file_os(&self, file_name: &OsStr) -> String {
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

I don't know if it's better to cfg-out that function on macos too, or just mark it with #[allow(dead_code)].

[lilyball]

I've included the above patch in the Nix PR for 0.15.1.

[eth-p]

Disabling the tests on MacOS is a working solution, but I felt like it still left some room for error (e.g. Linux with ZFS utf8only). I made a PR (https://github.com/sharkdp/bat/pull/985) that attempts to fix the underlying cause of this error by avoiding accessing the filesystem for some of the testing.

[sharkdp]

I suspect this is a test that simply always fails on macOS. It appears to be attempting to create a filename with invalid utf8, but macOS doesn't allow this.

Oh right. I ran into this with fd as well. I wonder why this hasn't been caught in a CI build (https://travis-ci.org/github/sharkdp/bat/builds/685790785)? Maybe the Travis OSX VMs use a file system that allows for invalid UTF-8 filenames...

Disabling the tests on MacOS is a working solution, but I felt like it still left some room for error (e.g. Linux with ZFS utf8only).

So how about just letting the test succeed in case the file could not be created? I think that would be a reasonable solution which would work on all operating systems.

I made a PR (#985) that attempts to fix the underlying cause of this error by avoiding accessing the filesystem for some of the testing.

Thanks. Let's discuss these changes in the PR.

[lilyball]

Maybe the Travis OSX VMs use a file system that allows for invalid UTF-8 filenames...

It looks like HFS+ allows invalid UTF-8 by transforming the filename. The name invalid_\xFEutf8_filename.rs ends up on-disk as a file named invalid_%FEutf8_filename.rs instead. APFS doesn't do this, it just rejects the filename.

[lilyball]

Confirmed fixed, thank you.