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

Add test cases for non-utf8 paths #11872

Closed
wants to merge 1 commit into from

Conversation

flaper87
Copy link
Contributor

closes #9406

@flaper87
Copy link
Contributor Author

@huonw r?

@@ -573,6 +575,16 @@ mod test {
}

#[test]
fn test_non_utf8_glob() {
let mut p = os::tmpdir();
p.push("\xe5");
Copy link
Member

Choose a reason for hiding this comment

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

This is still a UTF-8 path: it's just the 0xE5'th codepoint, i.e. two bytes (any and every string in Rust is UTF-8).

I guess .push([0xff]) may work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't push [0xff] unless it's converted to str which would end up on the same case I submitted. The point of this specific test is to create that directory and then call glob. Current version creates a directory named å under /tmp.

Copy link
Member

Choose a reason for hiding this comment

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

You can push things other than strings, and you have to push something other than a string for this to be a valid test, because otherwise you have a UTF-8 path.

Copy link
Member

Choose a reason for hiding this comment

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

(I guess it may have to be &[0xFF] or it may not be able to find the BytesContainer impl.)

@flaper87
Copy link
Contributor Author

not ready to be merged yet. The glob test fails! Will work on it later!

@lilyball
Copy link
Contributor

Yay, the glob test fails! That's because it's finally correct ;)

Glob itself doesn't match properly against UTF-8 pathnames. It ignores them, and that's because the code was written to use strings. Line 136 of libglob/lib.rs contains a // FIXME explaining that it's ignoring non-utf8 filenames but that it really should be able to match them against a *.

It's also possible that fs::readdir() doesn't handle utf-8 filenames properly either. I don't know if there are any tests specifically on that functionality. Maybe you should duplicate your test_non_utf8_glob and modify it for fs::readdir (and stick it in the right mod).

@lilyball
Copy link
Contributor

After investigating some more, my issue with fs::readdir() is a red herring. The actual problem I was experiencing is HFS+ doesn't allow non-unicode filenames, so the call to mkdir() was actually creating a folder named "%FF" instead of \xFF.

@flaper87
Copy link
Contributor Author

Since glob is still broken, I'll abandon this PR and submit one with tests just for fs::

I copied the glob test to the issue as a reference code for testing the fix to come.

@flaper87 flaper87 closed this Jan 29, 2014
@lilyball
Copy link
Contributor

New issue filed for glob() and non-utf8 paths as #11916.

@flaper87 flaper87 deleted the issue-9406 branch March 11, 2014 09:16
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2023
add lint against unit tests in doctests

During RustLab, Alice Ryhl brought to my attention that the Andoid team stumbled over the fact that if one attempts to write a unit test within a doctest, it will be summarily ignored. So this lint should help people wondering why their tests won't run.

---

changelog: New lint: [`test_attr_in_doctest`]
[rust-lang#11872](rust-lang/rust-clippy#11872)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants