search

imsnif / diskonaut

Terminal disk space navigator 🔭
MIT License
2.48k stars 66 forks source link

Different results from running the tests locally #42

Closed redzic closed 4 years ago

redzic commented 4 years ago

Previously discussed in #40

When I run the tests locally with cargo test on the main branch, I get very different results each time. Sometimes all the tests pass (which is somewhat rare), sometimes most of them fail.

Terminal screen capture: https://asciinema.org/a/MGtehVJYHt76Pq1DTDQyRxv2J

Raw output captured by script: output.txt

imsnif commented 4 years ago

So just to reiterate: this does not happen to me and does not happen in our CI. I have a guess as to what's causing this. @redzic, could you try changing this line: https://github.com/imsnif/diskonaut/blob/main/src/main.rs#L147 to .parallelism(::jwalk::Parallelism::Serial) and seeing if it solves the issue for you? This will change the HD scanning from multi-threaded to single threaded. Maybe running lots of tests in parallel is making things go out of whack for this.

If it doesn't, could you try and see if you can get the tests to fail by running just half of them (just comment out half), if so, just one or two tests?

Thanks for the help on this!

redzic commented 4 years ago

Wow... changing that line resolves the issue for me. All 37 tests pass now every single time. I wonder why the multi-threaded version works on other machines but not mine.

imsnif commented 4 years ago

Aha! That's great news. I can't completely explain why this is happening. This was kind of a gut feeling. I'd like to add conditional compilation for this (to make it be serial for tests and the way it is now for not-tests). Would you like to work on this, @redzic or shall I?

redzic commented 4 years ago

I think I'd like to work on this, but I'm not too familiar with conditional compilation.

Looking at the docs, it seems like I would need to do something like this:

.parallelism({
    if cfg!(test) {
        ::jwalk::Parallelism::Serial
    } else {
        RayonDefaultPool
    }
})

Does that look right? Perhaps I could import both Serial and RayonDefaultPool into scope so that each branch of the if statement looks the same.

redzic commented 4 years ago

Actually, it seems like you already do this in the code by having bools that change depending on cfg attributes. I think I'll just follow that pattern instead and open a PR.