--test-shard ignores benchmarks?

[killerswan]

Consider the following commands. First one shard:

rustx $ ./main_test --test --bench --test-shard 1.2

running 1 test
test core::addition_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

And then a second (well, empty):

rustx $ ./main_test --test --bench --test-shard 2.2

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

But there was a benchmark which neither of the two shards ran:

rustx $ ./main_test --test --bench

running 2 tests
test core::addition_works ... ok
test core::addition_benchmarked ... bench:         3 ns/iter (+/- 3)

test result: ok. 1 passed; 0 failed; 0 ignored; 1 measured

Sample code: https://github.com/killerswan/rustx/blob/07347184ded27a664544b40f032940efc339e340/src/rustx/core.rs

[emberian]

I can't make heads or tails of this.

extern crate test;

#[main]
fn say_hi () {
   println!("Hello!!");
}

#[test]
fn addition_works () {
   assert! (2 + 2 == 4);
}

#[test]
fn test2() {
    assert!(1 + 1 == 2);
}

#[bench]
fn addition_benchmarked (b: &mut test::Bencher) {
   let mut sum = 0;
   b.iter(|| sum += 1)
}

And the following patch:

diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs
index 4681c02..9517961 100644
--- a/src/libtest/lib.rs
+++ b/src/libtest/lib.rs
@@ -156,6 +156,20 @@ impl TestFn {
     }
 }

+impl fmt::Show for TestFn {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write(match *self {
+            StaticTestFn(..) => "StaticTestFn(..)",
+            StaticBenchFn(..) => "StaticBenchFn(..)",
+            StaticMetricFn(..) => "StaticMetricFn(..)",
+            DynTestFn(..) => "DynTestFn(..)",
+            DynMetricFn(..) => "DynMetricFn(..)",
+            DynBenchFn(..) => "DynBenchFn(..)"
+        }.as_bytes()).ok().unwrap();
+        Ok(())
+    }
+}
+
 /// Manager of the benchmarking runs.
 ///
 /// This is feed into functions marked with `#[bench]` to allow for
@@ -170,13 +184,14 @@ pub struct Bencher {

 // The definition of a single test. A test runner will run a list of
 // these.
-#[deriving(Clone)]
+#[deriving(Clone, Show)]
 pub struct TestDesc {
     pub name: TestName,
     pub ignore: bool,
     pub should_fail: bool,
 }

+#[deriving(Show)]
 pub struct TestDescAndFn {
     pub desc: TestDesc,
     pub testfn: TestFn,
@@ -242,15 +257,9 @@ pub fn test_main(args: &[StrBuf], tests: Vec<TestDescAndFn> ) {
 pub fn test_main_static(args: &[StrBuf], tests: &[TestDescAndFn]) {
     let owned_tests = tests.iter().map(|t| {
         match t.testfn {
-            StaticTestFn(f) =>
-            TestDescAndFn { testfn: StaticTestFn(f), desc: t.desc.clone() },
-
-            StaticBenchFn(f) =>
-            TestDescAndFn { testfn: StaticBenchFn(f), desc: t.desc.clone() },
-
-            _ => {
-                fail!("non-static tests passed to test::test_main_static");
-            }
+            StaticTestFn(f) => TestDescAndFn { testfn: StaticTestFn(f), desc: t.desc.clone() },
+            StaticBenchFn(f) => TestDescAndFn { testfn: StaticBenchFn(f), desc: t.desc.clone() },
+            _ => fail!("non-static tests passed to test::test_main_static")
         }
     }).collect();
     test_main(args, owned_tests)
@@ -739,10 +748,9 @@ pub fn fmt_bench_samples(bs: &BenchSamples) -> StrBuf {
 }

 // A simple console test runner
-pub fn run_tests_console(opts: &TestOpts,
-                         tests: Vec<TestDescAndFn> ) -> io::IoResult<bool> {
-    fn callback<T: Writer>(event: &TestEvent,
-                           st: &mut ConsoleTestState<T>) -> io::IoResult<()> {
+pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn> ) -> io::IoResult<bool> {
+
+    fn callback<T: Writer>(event: &TestEvent, st: &mut ConsoleTestState<T>) -> io::IoResult<()> {
         match (*event).clone() {
             TeFiltered(ref filtered_tests) => st.write_run_start(filtered_tests.len()),
             TeWait(ref test, padding) => st.write_test_start(test, padding),
@@ -778,6 +786,7 @@ pub fn run_tests_console(opts: &TestOpts,
             }
         }
     }
+
     let mut st = try!(ConsoleTestState::new(opts, None::<StdWriter>));
     fn len_if_padded(t: &TestDescAndFn) -> uint {
         match t.testfn.padding() {
@@ -933,9 +942,7 @@ fn get_concurrency() -> uint {
     }
 }

-pub fn filter_tests(
-    opts: &TestOpts,
-    tests: Vec<TestDescAndFn> ) -> Vec<TestDescAndFn> {
+pub fn filter_tests(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> Vec<TestDescAndFn> {
     let mut filtered = tests;

     // Remove tests that don't match the test filter
@@ -972,6 +979,7 @@ pub fn filter_tests(
     match opts.test_shard {
         None => filtered,
         Some((a,b)) => {
+            println!("sharding over: {}", filtered);
             filtered.move_iter().enumerate()
             .filter(|&(i,_)| i % b == a)
             .map(|(_,t)| t)

I get:

sharding over: [TestDescAndFn { desc: TestDesc { name: addition_benchmarked, ignore: false, should_fail: false }, testfn: StaticBenchFn(..) }, TestDescAndFn { desc: TestDesc { name: addition_works, ignore: false, should_fail: false }, testfn: StaticTestFn(..) }, TestDescAndFn { desc: TestDesc { name: test2, ignore: false, should_fail: false }, testfn: StaticTestFn(..) }]

But even ./foo --test-shard 1.1, which should in theory run everything, is running nothing!

[huonw]

IIRC, indexing starts from 0. e.g. 0.1 runs everything, 0.2 and 1.2 run half each.

(We should have an out-of-bounds warning...)

(Or maybe not? I can't remember the syntax.)

[emberian]

@killerswan the issue is actually quite simpler. With two shards, your choices are 0.2 and 1.2, not 2.2! That will check if i % 2 == 2, which is obviously never going to be true. I found this very counter intuitive. I'll change it to behavior more logically (1.2 and 2.2 instead of 0.2 and 1.2)