tokio-rs / loom

Concurrency permutation testing tool for Rust.
MIT License
2.14k stars 111 forks source link

Fix readme example #132

Closed faern closed 4 years ago

faern commented 4 years ago

The example in the readme did not compile:

th.join().unwrap();
   ^^^^ method not found in `()`

The first commit fixes this by removing a semicolon so the thread join handle becomes the value of the iterator.

Secondly. I was pulling my hair for quite a while trying to understand why the test would not fail. It had a bug, why did loom not find it? Because the test was marked as #[should_panic] of course. Is it not more pedagogical to have a buggy test fail, to show what loom does when it finds an error? 🤔

paulkernfeld commented 4 years ago

Thanks for submitting this! I had the same issue, and removing the semicolon did indeed fix it.

I would recommend splitting the #[should_panic] removal into a separate PR, since that change might require some more discussion. I would guess that whoever wrote the test would say that the #[should_panic] annotation is their preferred way to indicate to the reader of the code that the test is expected to fail.

faern commented 4 years ago

Ok. Removed that. Getting the compile error fixed is better than not being merged.

faern commented 4 years ago

Ping? This should be a non-controversial and easily reviewed PR making it possible to actually copy and paste the example without hitting errors.

jxs commented 4 years ago

thanks!