Fix a bug where last part of the key is discarded and replaced with default filename

[nsmaciej]

Before 1.0.0, this line used to say

Some(name) if name.len() > 0 => {

With 1.0.0 it was changed to if name.is_empty() which caused the filename part of the path to be discarded instead of prepending it to PREFS_FILE_EXTENSION

[nsmaciej]

I wasn't expecting any failures. Looks like something to do with app_dir. When test_save_load fails you can see it:

Err(Directory(Io(Error { repr: Os { code: 183, message: "Cannot create a file when that file already exists." } })))

I cannot reproduce the error. Works on my mac™.

[andybarron]

Hahaaaa that is a terrible bug, whoops. Thanks for doing this.

As for the raciness, is there a way to make Cargo run tests in series rather than parallel? It would be nice not to glob all our tests together...

[nsmaciej]

I realised it must be a race condition. I remove the second test in my fork and it got rid of the problem.

I don't think globing the test cases is a good idea either, it feels like just hiding the problem.

It might be possible to special case this using the error codes. It also feels quite std bug-worthy, "file already exists" doesn't feel like something DirBuilder should be returning.

[nsmaciej]

There is a bug already opened for this rust-lang/rust#33707.
I feel it's ok then to glob the test case for now and release a new version.

If that's ok with you I'll clean this up and make it use gen_sample_prefs again.

[andybarron]

Sounds great, thank you! 😁

…

On Mon, Feb 6, 2017 at 6:45 PM Maciej Goszczycki ***@***.***> wrote: There is a bug already opened for this rust-lang/rust#33707 <rust-lang/rust#33707>. I feel it's ok then to glob the test case for now and release a new version. If that's ok with you I'll clean this up and make it use gen_sample_prefs again. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#17 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADdko4PwpbdFeNfHkaX5y_Vv1WVxq1-Qks5rZ9rWgaJpZM4L42Hf> .

[nsmaciej]

I think this looks fine

[andybarron]

Wonderful, merging now. I'll publish 1.0.1 and yank 1.0.0 as soon as I get the chance.