Merge pull request sourcefrog#263 from sourcefrog/mutants

Better mutants testing

.cargo/mutants.toml

@@ -7,21 +7,6 @@ exclude_re = [
     "replace <impl Iterator for IterStitchedIndexHunks>::next -> Option<Self::Item> with Some\\(Default::default\\(\\)\\)",
 ]
 
-# examine_globs = [
-#     # "src/backup.rs",
-#     # "src/bandid.rs",
-#     "src/blockdir.rs",
-#     "src/blockhash.rs",
-#     "src/bin/conserve.rs",
-#     "src/change.rs",
-#     "src/counters.rs",
-#     "src/jsonio.rs",
-#     "src/restore.rs",
-#     "src/stitch.rs",
-#     # "src/transport.rs",
-#     "src/transport/local.rs",
-# ]
-
 # Include only files that are currently well-tested; most of the others should
 # be tested eventually (https://github.com/sourcefrog/conserve/issues/241.
 #
@@ -34,9 +19,24 @@ exclude_re = [
 # text format might change.
 #
 # I test for mutants on Unix so skip Windows code.
+examine_globs = [
+    "src/apath.rs",
+    "src/bandid.rs",
+    "src/blockdir.rs",
+    "src/blockhash.rs",
+    "src/change.rs",
+    "src/counters.rs",
+    "src/jsonio.rs",
+    "src/stitch.rs",
+    # "src/restore.rs", # really close but not quite covered
+    #     # "src/backup.rs",
+    #     "src/bin/conserve.rs",
+    "src/transport.rs",
+    #     "src/transport/local.rs",
+]
+
 exclude_globs = [
     "backup.rs",            # almost well tested but some gaps
-    "bandid.rs",            # almost well tested but some gaps
     "metric_recorder.rs",
     "progress.rs",
     "src/progress/term.rs",

.github/workflows/install.yml

@@ -1,3 +1,8 @@
+# Check install from crates.io works, both with and without --locked.
+#
+# This is mostly to check for loose dependencies that might have broken
+# the build after it was released.
+
 on:
   push:
     paths:
@@ -16,4 +21,4 @@ jobs:
     steps:
       - name: cargo-install
         run: |
-          cargo install cargo-mutants ${{ matrix.locked }} --features=${{ matrix.features }}
+          cargo install cargo-mutants ${{ matrix.locked }} --features=${{ matrix.features }}

.github/workflows/rust.yml

@@ -26,7 +26,27 @@ env:
 # TODO: Add -D warnings when that's clean on Windows.
 
 jobs:
+  # Run tests just on Ubuntu to establish that things basically work,
+  # before launching mutants etc
+  quick-test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: stable
+      - uses: swatinem/rust-cache@v2
+      - name: Show version
+        run: |
+          rustup show
+          cargo --version
+          rustc --version
+      - name: Test
+        run: >
+          cargo test --features fail/failpoints
+
   tests:
+    needs: [quick-test]
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: true
@@ -110,7 +130,7 @@ jobs:
   pr-mutants:
     runs-on: ubuntu-latest
     if: github.event_name == 'pull_request'
-    needs: [tests]
+    needs: [quick-test]
     steps:
       - uses: actions/checkout@v4
         with:
@@ -129,11 +149,46 @@ jobs:
           tool: cargo-mutants
       - name: Mutants in diff
         run: >
-          cargo mutants --no-shuffle -vV --in-diff git.diff -- --features
+          cargo mutants --no-shuffle -vV --in-diff git.diff --in-place -- --features
           fail/failpoints
       - name: Archive mutants.out
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: mutants-incremental.out
           path: mutants.out
+
+  cargo-mutants:
+    runs-on: ubuntu-latest
+    needs: [quick-test]
+    strategy:
+      # We want to see all the missed mutants so don't fail fast.
+      fail-fast: false
+      matrix:
+        shard: [0, 1, 2, 3]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - uses: taiki-e/install-action@v2
+        name: Install cargo-mutants using install-action
+        with:
+          tool: cargo-mutants
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: stable
+      - uses: swatinem/rust-cache@v2
+      - name: Run mutant tests
+        # Don't use the S3 features because they require AWS credentials for realistic
+        # testing.
+        run: |
+          cargo mutants --no-shuffle -vV --cargo-arg=--no-default-features \
+            --in-place \
+            --baseline=skip --shard ${{ matrix.shard }}/4 \
+            -- \
+            --features fail/failpoints
+      - name: Archive results
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: mutants-${{ matrix.shard }}.out
+          path: mutants.out

src/apath.rs

@@ -139,12 +139,6 @@ impl From<Apath> for String {
     }
 }
 
-impl<'a> From<&'a Apath> for &'a str {
-    fn from(a: &'a Apath) -> &'a str {
-        &a.0
-    }
-}
-
 impl<'a> From<&'a str> for Apath {
     fn from(s: &'a str) -> Apath {
         assert!(Apath::is_valid(s), "invalid apath: {s:?}");
@@ -317,6 +311,47 @@ mod test {
         assert_eq!(apath.to_string(), "/something");
     }
 
+    #[test]
+    fn apath_eq_str() {
+        let apath: Apath = "/something".parse().unwrap();
+        assert_eq!(apath, "/something");
+        assert_ne!(apath, "/something/else");
+    }
+
+    #[test]
+    fn str_eq_apath() {
+        let apath: Apath = "/something".parse().unwrap();
+        assert_eq!("/something", apath);
+        assert_ne!("/something/else", apath);
+    }
+
+    #[test]
+    fn eq_apath() {
+        let apath: Apath = "/something".parse().unwrap();
+        assert_eq!(apath, Apath::from("/something"));
+        assert_ne!(apath, Apath::from("/something/else"));
+    }
+
+    #[test]
+    fn asref_str() {
+        let apath: Apath = "/something".parse().unwrap();
+        let s: &str = apath.as_ref();
+        assert_eq!(s, "/something");
+    }
+
+    #[test]
+    fn display_apath() {
+        let apath: Apath = "/something".parse().unwrap();
+        assert_eq!(format!("{}", apath), "/something");
+    }
+
+    #[test]
+    fn str_from_apath() {
+        let apath: Apath = "/something".parse().unwrap();
+        let s: &str = apath.as_ref();
+        assert_eq!(s, "/something");
+    }
+
     #[test]
     fn is_prefix_of() {
         use std::ops::Not;

src/bandid.rs

@@ -49,6 +49,9 @@ impl BandId {
     ///
     /// Currently only implemented for top-level bands.
     #[must_use]
+    #[mutants::skip]
+    // TODO: Mutating this currently causes a hang; it ought to be fixed by changing stitch to
+    // list the bands that are present rather than repeatedly subtracting.
     pub fn previous(&self) -> Option<BandId> {
         if self.0 == 0 {
             None