Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment with mutation testing #63

Closed
Notgnoshi opened this issue Nov 29, 2024 · 3 comments · Fixed by #70
Closed

Experiment with mutation testing #63

Notgnoshi opened this issue Nov 29, 2024 · 3 comments · Fixed by #70

Comments

@Notgnoshi
Copy link
Owner

Notgnoshi commented Nov 29, 2024

cargo mutants --in-place

--in-place is necessary because the tests operate on Herostratus itself as a Git repository, and use paths relative to the test CWD to do so.

MISSED   herostratus/src/git/rev.rs:21:36: replace | with & in walk in 0.7s build + 4.5s test
MISSED   herostratus/src/achievement/achievement.rs:120:9: replace LoggedRule::log_achievement with () in 0.7s build + 2.6s test
MISSED   herostratus/src/rules/h004_non_unicode.rs:18:9: replace <impl Rule for NonUnicode>::description -> &'static str with "xyzzy" in 0.7s build + 2.6s test
MISSED   herostratus/src/commands/fetch_all.rs:29:12: delete ! in fetch_all in 0.7s build + 2.6s test
MISSED   herostratus/src/rules/h002_shortest_subject_line.rs:47:9: replace <impl Rule for ShortestSubjectLine>::name -> &'static str with "xyzzy" in 0.7s build + 2.5s test
MISSED   herostratus/src/rules/h003_longest_subject_line.rs:48:9: replace <impl Rule for LongestSubjectLine>::description -> &'static str with "xyzzy" in 0.7s build + 2.6s test
MISSED   herostratus/src/rules/mod.rs:38:36: replace == with != in builtin_rules in 0.7s build + 2.5s test
MISSED   herostratus/src/achievement/achievement.rs:141:12: delete ! in LoggedRule::finalize_log in 0.7s build + 2.6s test
MISSED   herostratus/src/rules/mod.rs:37:37: replace == with != in builtin_rules in 0.7s build + 2.5s test
TIMEOUT  herostratus/src/achievement/process_rules.rs:60:28: replace += with *= in Achievements<'repo, Oids>::get_next_achievement_online in 0.6s build + 20.0s test
MISSED   herostratus/src/git/clone.rs:51:5: replace remove_dir_contents -> std::io::Result<()> with Ok(()) in 0.7s build + 2.6s test
MISSED   herostratus/src/rules/h001_fixup.rs:30:9: replace <impl Rule for Fixup>::description -> &'static str with "xyzzy" in 0.6s build + 2.5s test
MISSED   herostratus/src/rules/mod.rs:39:31: replace == with != in builtin_rules in 0.7s build + 2.5s test
MISSED   herostratus/src/rules/h002_shortest_subject_line.rs:44:9: replace <impl Rule for ShortestSubjectLine>::human_id -> &'static str with "xyzzy" in 0.7s build + 2.5s test
MISSED   herostratus/src/commands/remove.rs:7:5: replace remove -> eyre::Result<()> with Ok(()) in 0.7s build + 2.5s test
MISSED   herostratus/src/rules/h003_longest_subject_line.rs:45:9: replace <impl Rule for LongestSubjectLine>::name -> &'static str with "xyzzy" in 0.6s build + 2.5s test
MISSED   herostratus/src/rules/h002_shortest_subject_line.rs:57:31: replace < with == in <impl Rule for ShortestSubjectLine>::process in 0.7s build + 2.5s test
MISSED   herostratus/src/git/clone.rs:155:31: replace == with != in fetch_remote in 0.7s build + 2.5s test
MISSED   herostratus/src/git/clone.rs:159:25: replace += with *= in fetch_remote in 0.7s build + 2.5s test
MISSED   herostratus/src/achievement/process_rules.rs:103:45: replace += with *= in <impl Iterator for Achievements<'repo, Oids>>::next in 0.7s build + 2.5s test
MISSED   herostratus/src/config/config.rs:97:8: delete ! in write_config in 0.7s build + 2.4s test
MISSED   herostratus/src/rules/h001_fixup.rs:24:9: replace <impl Rule for Fixup>::human_id -> &'static str with "xyzzy" in 0.7s build + 2.6s test
MISSED   herostratus/src/rules/h001_fixup.rs:27:9: replace <impl Rule for Fixup>::name -> &'static str with "xyzzy" in 0.7s build + 2.4s test
MISSED   herostratus/src/rules/mod.rs:36:27: replace == with != in builtin_rules in 0.7s build + 2.5s test
MISSED   herostratus/src/git/clone.rs:122:5: replace fetch_remote -> eyre::Result<()> with Ok(()) in 0.8s build + 2.5s test
MISSED   herostratus/src/rules/h002_shortest_subject_line.rs:50:9: replace <impl Rule for ShortestSubjectLine>::description -> &'static str with "xyzzy" in 0.6s build + 2.6s test
MISSED   herostratus/src/achievement/process_rules.rs:51:44: replace += with *= in Achievements<'repo, Oids>::get_next_achievement_online in 0.7s build + 2.5s test
MISSED   herostratus/src/git/clone.rs:148:58: replace == with != in fetch_remote in 0.7s build + 2.5s test
MISSED   herostratus/src/achievement/achievement.rs:113:9: replace Rule::finalize -> Vec<Achievement> with vec![] in 0.7s build + 2.6s test
MISSED   herostratus/src/achievement/process_rules.rs:94:45: replace += with *= in <impl Iterator for Achievements<'repo, Oids>>::next in 0.7s build + 2.5s test
MISSED   herostratus/src/git/rev.rs:21:36: replace | with ^ in walk in 0.6s build + 2.5s test
150 mutants tested in 3m 42s: 30 missed, 88 caught, 31 unviable, 1 timeouts
@Notgnoshi
Copy link
Owner Author

cargo-mutants seems to not let you run --ignored tests, so there's a bunch of hits that should be excluded

rg -l '#\[ignore' | xargs sed -Ei 's|#\[ignore|// #[ignore|'

@Notgnoshi
Copy link
Owner Author

Notgnoshi commented Nov 29, 2024

This adds quite a bit of time to each test, but it resolves #64 well enough that cargo-mutants can run the integration tests

diff --git a/herostratus-tests/src/cmd.rs b/herostratus-tests/src/cmd.rs
index d96ce29..1a822e0 100644
--- a/herostratus-tests/src/cmd.rs
+++ b/herostratus-tests/src/cmd.rs
@@ -18,6 +18,14 @@ pub fn herostratus(data_dir: Option<&Path>) -> (Command, Option<TempDir>) {
         (Some(temp), data_dir)
     };

+    // This will slow down the integration tests
+    Command::new("cargo")
+        .arg("build")
+        .arg("-p")
+        .arg("herostratus")
+        .output()
+        .expect("Failed to build Herostratus");
+
     let mut cmd = Command::new(&*HEROSTRATUS);
     cmd.arg("--log-level=DEBUG").arg("--data-dir").arg(path);

@Notgnoshi
Copy link
Owner Author

Without integration tests

  • 160 mutants
  • 53 missed
  • 69 caught
  • 37 unviable
  • 2:04 execution time

With integration tests

cargo mutants --in-place --package herostratus --test-workspace true
  • 150 mutants
  • 30 missed
  • 88 caught
  • 31 unviable
  • 3:45 execution time

So it appears that the integration tests do indeed improve test coverage! (expected result ...)

Notgnoshi added a commit that referenced this issue Nov 30, 2024
Notgnoshi added a commit that referenced this issue Dec 30, 2024
Mutation testing revealed a few gaps that I thought were worth filling
in, and in doing so, it importantly highlighted a bug in
herostratus::git::pull_branch. This alone makes it worth it!

Closes #63
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant