Ad-hoc or scheduled mutation based testing #14589

Activity

1. alamb commented on Feb 11, 2025

   

   alamb

   on Feb 11, 2025

   Contributor

   More actions

   Hi @edmondop -- this sounds cool.

   I don't fully understand what you have in mind for mutation testing -- it seems like mutation testing is designed to find holes in our test coverage (a good goal). However, I am worried about adding an automatic test like unless someone has the bandwidth to monitor and fix issues it finds.

   We are already struggling to keep extended test running cleanly, for example

   • Extended tests are (still) failing on main #14576

   I worry if we added automatic mutation testing the results would simply be ignored -- in which case adding the tests doesn't add much value

   Perhaps you can try to run mutation tests manually, look at the results, and see if they uncover anything good
2. edmondop commented on Feb 11, 2025

   

   edmondop

   on Feb 11, 2025

   ContributorAuthor

   More actions

   @alamb they did, although they went out of space! If you click on the "this" hyperlink in the text of the issue, you get here https://github.com/edmondop/arrow-datafusion/actions/runs/13248520757/job/36980712855

   before it run out of memory, it output

   MISSED   datafusion/functions/src/math/log.rs:222:61: replace == with != in <impl ScalarUDFImpl for LogFunc>::simplify in 84.2s build + 6.9s test
   MISSED   datafusion/functions/src/lib.rs:179:5: replace register_all -> Result<()> with Ok(()) in 18.1s build + 5.7s test
   MISSED   datafusion/physical-optimizer/src/join_selection.rs:227:9: replace || with && in try_collect_left in 77.1s build + 0.4s test
   MISSED   datafusion/physical-plan/src/aggregates/mod.rs:1278:27: replace <= with > in group_id_array in 90.6s build + 41.3s test
   MISSED   datafusion/physical-optimizer/src/limit_pushdown.rs:77:9: replace <impl PhysicalOptimizerRule for LimitPushdown>::name -> &str with "" in 14.8s build + 0.3s test
   MISSED   datafusion/functions-aggregate/src/array_agg.rs:383:13: replace - with + in <impl Accumulator for DistinctArrayAggAccumulator>::size in 70.4s build + 1.7s test
   MISSED   datafusion/functions-aggregate/src/sum.rs:469:9: replace <impl Accumulator for DistinctSumAccumulator<T>>::size -> usize with 1 in 35.2s build + 1.6s test
   MISSED   datafusion/functions-nested/src/concat.rs:292:35: delete ! in <impl ScalarUDFImpl for ArrayConcat>::return_type in 40.2s build + 5.2s test
   MISSED   datafusion/proto-common/src/to_proto/mod.rs:247:9: replace <impl TryFrom for protobuf::Schema>::try_from -> Result<Self, Self::Error> with Ok(Default::default()) in 14.9s build + 2.7s test
   MISSED   datafusion/optimizer/src/eliminate_duplicated_expr.rs:45:9: replace <impl PartialEq for SortExprWrapper>::eq -> bool with true in 46.4s build + 11.5s test
   MISSED   datafusion/functions-aggregate/src/string_agg.rs:176:9: replace <impl Accumulator for StringAggAccumulator>::size -> usize with 0 in 41.0s build + 1.6s test
   MISSED   datafusion/physical-plan/src/aggregates/group_values/single_group_by/bytes.rs:77:9: replace <impl GroupValues for GroupValuesByes<O>>::size -> usize with 0 in 23.8s build + 39.4s test
   MISSED   datafusion/catalog/src/memory.rs:166:9: replace <impl SchemaProvider for MemorySchemaProvider>::table_names -> Vec<String> with vec!["xyzzy".into()] in 25.3s build + 0.6s test
   MISSED   datafusion/expr-common/src/type_coercion/aggregates.rs:276:5: replace is_covariance_support_arg_type -> bool with false in 8.7s build + 5.7s test
   MISSED   datafusion/physical-expr/src/window/window_expr.rs:294:20: delete ! in AggregateWindowExpr::get_result_column in 38.3s build + 9.0s test
   TIMEOUT  datafusion/physical-plan/src/aggregates/row_hash.rs:1059:9: replace GroupedHashAggregateStream::update_merged_stream -> Result<()> with Ok(()) in 53.6s build + 300.0s test
   MISSED   datafusion/physical-expr-common/src/sort_expr.rs:429:9: replace LexOrdering::from_lex_requirement -> LexOrdering with Default::default() in 7.7s build + 10.2s test
   MISSED   datafusion/sql/src/statement.rs:1607:69: replace - with / in SqlToRel<'_, S>::show_variable_to_plan in 45.2s build + 9.4s test

   Some of these are not not to be worried about, honestly. For example

   impl UDF for MyFunc {
         fn func_name(self) -> String {
              // I will never test this, so mutating this code to return "bar" will make the mutant survive 
               return "foo".to_string()
         }
   }
   

   Cargo mutants provide different ways of skipping mutations. You can put an attribute you can put on "stuff that you don't want to be mutate".

   impl UDF for MyFunc {
         #[cfg_attr(test, mutants::skip)]
         fn func_name(self) -> String {
              // I will never test this, so mutating this code to return "bar" will make the mutant survive 
               return "foo".to_string()
         }
   }

   or using exclusions (see mutants.toml)

   exclude_re = [" <impl Accumulator for .*>::size "]
3. 2010YOUY01 commented on Feb 12, 2025

   

   2010YOUY01

   on Feb 12, 2025

   Member

   More actions

   @alamb they did, although they went out of space! If you click on the "this" hyperlink in the text of the issue, you get here https://github.com/edmondop/arrow-datafusion/actions/runs/13248520757/job/36980712855

   before it run out of memory, it output

   Looks like it's running out of disk space due to lots of recompilations
   The default CI runner's disk budget is < 20GB

   👍React with 👍1Reacted by Andrew Lamb