Test on all operating systems and make Miri run on CI

[philippeitis]

It might be a good idea to test on all operating systems, since they have differences in memory management and file operations which might cause bugs. It should run the tests in parallel, so there shouldn't be any significant overhead for this change.

This also fixes the set_env command to make Miri actually run (though I haven't fixed any bugs, so it still fails, but now it does so because it actually runs).

[codecov-io]

Codecov Report

Merging #90 (2b0d67e) into master (c208619) will increase coverage by 1.48%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #90      +/-   ##
==========================================
+ Coverage   91.37%   92.85%   +1.48%     
==========================================
  Files           5        5              
  Lines         313      308       -5     
==========================================
  Hits          286      286              
+ Misses         27       22       -5     

Impacted Files	Coverage Δ
src/backend/path.rs	100.00% <0.00%> (+16.12%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c208619...2b0d67e. Read the comment docs.

[TheNeikos]

I like those changes! Any ideas why windows fails?

Regarding the miri fail, this is due to a YAML dependency and is also tracked here https://github.com/TheNeikos/rustbreak/issues/87

[philippeitis]

It seems to be an issue with tempfiles, since it's not possible to persist the database file due to Access is denied. This might be because taking the path and converting it to owned drops the temp file too soon, though I'm not actually certain about this (one possibly related issue?, another possibly related issue?). Manually creating a file seems to work without any problems - for instance, replacing test_path_backend_existing() with the following works, and tests the same behaviour (though, it's not as neat).

    #[test]
    #[cfg_attr(miri, ignore)]
    fn test_path_backend_existing() {
        let path = PathBuf::from("./test_path_backend_existing.db");
        {
            let file = std::fs::OpenOptions::new()
                .read(true)
                .write(true)
                .truncate(true)
                .create(true)
                .open(&path).expect("could not create temporary file");
        }
        let (mut backend, existed) = PathBackend::from_path_or_create(path.clone()).expect("could not create backend");
        assert!(existed);
        let data = [4, 5, 1, 6, 8, 1];

        backend.put_data(&data).expect("could not put data");
        assert_eq!(backend.get_data().expect("could not get data"), data);
        std::fs::remove_file(path).expect("file should be deleted");
    }

[niluxv]

Nice. I hadn't noticed that miri did not run.

Strange windows issue. I'm afraid it will also cause issues with the integration tests. The following already triggers the error:

fn test_file_persist() {
    let file = NamedTempFile::new().expect("could not create temporary file");
    println!("`file` path: {:?}", file.path());
    let file_path = file.path().to_owned();
    let file2 = NamedTempFile::new().expect("could not create temporary file");
    println!("`file2` path: {:?}", file2.path());
    file2.persist(file_path).expect("could not persist file2 to file");
}

Maybe the problem is that file is open (locked?) when we try to persist a second tempfile to it's path (file.path())?

@TheNeikos How about merging this and creating a tracking issue for the windows test failures?

[niluxv]

Maybe the problem is that file is open (locked?) when we try to persist a second tempfile to it's path (file.path())?

I tested it on windows, and this is exactly the problem. I can make a PR to fix the tests. Maybe this PR can be merged first to keep the git history somewhat clean?

[niluxv]

I went ahead and opened #92 to fix the tests on windows (and document the YAML UB).

[niluxv]

@TheNeikos Any chance for these two PRs to be merged?