PIR todo() is frequently misused

[Util]

In pure PIR .t tests that use the test_more harness, the todo() function either:
* is constant todo(0, ...), preventing "unexpected success" info from ever printing, or
* is constant todo(1, ...), causing confusing "unexpected success" to always happen on TODOed platforms, or
* is conditioned todo($I0, ...), causing duplication of code or inability to use is() and friends.
This is a result of the todo() function's documentation and API.

Further analysis to follow...

Originally http://trac.parrot.org/parrot/ticket/1302

[Util]

A refresher from Perl 5 testing: TODO and SKIP are similar concepts, but they have distinct uses.

• TODO is used when a test _should_ pass, but is known to not pass (yet) in some or all circumstances.
• SKIP is used when a test should not or cannot run at all, due to circumstances outside the control of the testing code (and coders).

Either SKIP or TODO will prevent a FAILure due to false positives, but TODO has another role; since the TODO-ed test actually runs, the test harness can communicate that the test "succeeded unexpectedly". This success is an important piece of info.

[Util]

Misleading documentation is here:

• examples/tutorial/90_writing_tests.pir

  todo(1, 42, "todo test")
  

• runtime/parrot/library/Test/More.pir

  todo(0, 'this is a failed test', 'reason for todo')
  

[Util]

Instances I know of:

1. Conditioned, therefore correct, but non-optimal; the first 4 abandon is() and similar functions to avoid duplicate code:
   • t/compilers/imcc/syn/subflags.t
   • t/library/p6object.t
   • t/pmc/array.t
   • t/pmc/packfile.t
   • t/pmc/complex.t - uses todo() via a macro, and still has to duplicate two tests!

2. Mixed usage:
   • t/oo/metamodel.t
     1. Correct via conditioning
     2. Misused via 0

3. Hardcoded 1 - all need to have conditioning logic:
   • t/op/arithmetics.t
   • t/op/io.t
   • t/op/number.t
   • t/pmc/float.t

4. Hardcoded 0 - these are all special cases:
   • t/pmc/class.t - This almost gets it right, via exception handler. I think the current code will produce an extra test run if the code starts succeeding.
   • t/pmc/default.t - Needs to use an exception handler, similar to class.t
   • t/op/inf_nan.t - where the ops are not implemented, the code must be commented out, or the PIR will mis-parse. Their todo()s should be skip()s.

[bubaflub]

5762 byte attachment from bubaflub
at http://trac.parrot.org/parrot/raw-attachment/ticket/1302/t_op_inf_nan_t.patch

index e31db51..4a94d7d 100644
--- t/op/inf_nan.t
+++ t/op/inf_nan.t
@@ -234,53 +234,53 @@ Tests for mathematical operations with Inf and Nan.
     $N0 = 'Inf'
     #$N1 = cot $N0
     #is($N1, 'NaN', 'cot: cot Inf')
-    todo(0, 'cot Inf', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = '-Inf'
   #$N1 = cot $N0
   #is($N1, 'NaN', '... cot -Inf')
-    todo(0, 'cot -Inf', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = 'NaN'
   #$N1 = cot $N0
   #is($N1, 'NaN', '... cot NaN')
-    todo(0, 'cot NaN', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
  .end
  
  .sub test_coth
     $N0 = 'Inf'
     #$N1 = coth $N0
     #is($N1, 1, 'coth: coth Inf')
-    todo(0, 'coth Inf', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = '-Inf'
   #$N1 = coth $N0
   #is($N1, -1, '... coth -Inf')
-    todo(0, 'coth -Inf', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = 'NaN'
   #$N1 = coth $N0
   #is($N1, 'NaN', '... coth NaN')
-    todo(0, 'coth NaN', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
  .end
  
  .sub test_acot 
     $N0 = 'Inf'
     #$N1 = acot $N0
     #is($N1, 'NaN', 'acot: acot Inf')
-    todo(0, 'acot Inf', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = '-Inf'
   #$N1 = acot $N0
   #is($N1, 'NaN', '... acot -Inf')
-    todo(0, 'acot -Inf', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = 'NaN'
   #$N1 = acot $N0
   #is($N1, 'NaN', '... acot NaN')
-    todo(0, 'acot NaN', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = '-2'
   #$N1 = acot $N0
   #is($N1, 'NaN', '... acot -2')
-    todo(0, 'acot -2', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
   $N0 = '2'
   #$N1 = acot $N0
   #is($N1, 'NaN', '... acot 2')
-    todo(0, 'acot 2', 'cot/coth/acot not implemented for real numbers')
-    skip(1, 'cot/coth/acot not implemented for real numbers')
  .end
  
  .sub test_sec
  @@ -416,19 +416,19 @@ Tests for mathematical operations with Inf and Nan.
   $N0 = 'Inf'
   $I0 = floor $N0
   #is($I0, 'Inf', 'floor Inf')
-    todo(0, 'floor Inf', 'rounding nan/inf gives something like -2147483648')
-    skip(1, 'rounding nan/inf gives something like -2147483648')
   $N0 = 'NaN'
   $I0 = floor $N0
   #is($I0, 'NaN', 'floor Inf')
-    todo(0, 'floor NaN', 'rounding nan/inf gives something like -2147483648')
-    skip(1, 'rounding nan/inf gives something like -2147483648')
   $N0 = 'Inf'
   $I0 = ceil $N0
   #is($I0, 'Inf', 'floor Inf')
-    todo(0, 'ceil Inf', 'rounding nan/inf gives something like -2147483648')
-    skip(1, 'rounding nan/inf gives something like -2147483648')
   $N0 = 'NaN'
   $I0 = ceil $N0
   #is($I0, 'NaN', 'floor Inf')
-    todo(0, 'ceil NaN', 'rounding nan/inf gives something like -2147483648')
-    skip(1, 'rounding nan/inf gives something like -2147483648')
  .end
  
  .sub test_nan_complex
  @@ -437,7 +437,7 @@ Tests for mathematical operations with Inf and Nan.
   set $P1, "1 + i"
   $P1 += $N0
   #is($P1, 'NaN', '1+i + NaN')
-    todo(0, '1+i + NaN should be NaN')
-    skip(1, '1+i + NaN should be NaN')
  .end
  
  .sub test_fdiv_integer_pmc_nan
  @@ -447,7 +447,7 @@ Tests for mathematical operations with Inf and Nan.
   $N0 = 'NaN'
   fdiv $P1, $P2, $N0
   #is($P1, 'NaN', 'fdiv with Integer PMCs and NaN')
-    todo(0, 'fdiv with Integer PMCs and NaN', 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
-    skip(1, 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
  .end
  
  .sub test_fdiv_float_pmc_nan
  @@ -457,7 +457,7 @@ Tests for mathematical operations with Inf and Nan.
   $N0 = 'NaN'
   fdiv $P1, $P2, $N0
   #is($P1, 'NaN','fdiv with Float PMCs and NaN')
-    todo(0,'fdiv with Float PMCs and NaN', 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
-    skip(1, 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
  .end
  
  .sub test_fdiv_float_integer_pmc_nan
  @@ -467,7 +467,7 @@ Tests for mathematical operations with Inf and Nan.
   $N0 = 'NaN'
   fdiv $P1, $P2, $N0
   #is($P1, 'NaN', 'fdiv with Float and Integer PMCs and NaN')
-    todo(0, 'fdiv with Float and Integer PMCs and NaN', 'fdiv/mod/cmod do not play nicely with PMCs and NaN')  
-    skip(1, 'fdiv/mod/cmod do not play nicely with PMCs and NaN')  
  .end
  
  .sub test_cmod_float_integer_pmc_nan
  @@ -477,7 +477,7 @@ Tests for mathematical operations with Inf and Nan.
   $N0 = 'NaN'
   cmod $P1, $P2, $N0
   #is($P1, 'NaN', 'cmod with Float and Integer PMCs and NaN')
-    todo(0, 'cmod with Float and Integer PMCs and NaN', 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
-    skip(1, 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
  .end
  
  .sub test_mod_float_integer_pmc_nan
  @@ -487,7 +487,7 @@ Tests for mathematical operations with Inf and Nan.
   $N0 = 'NaN'
   mod $P1, $P2, $N0
   #is($P1, 'NaN', 'mod with Float and Integer PMCs and NaN')
-    todo(0, 'mod with Float and Integer PMCs and NaN', 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
-    skip(1, 'fdiv/mod/cmod do not play nicely with PMCs and NaN')
  .end
  
  # Local Variables:

[bubaflub]

Is this what you were looking for?

[bubaflub]

patch to change todo to skip in t/op/inf_nan.t

[jkeenan]

Util: Does bubaflub's patch satisfy your concerns as expressed in item 4, 3rd bullet point above?

Thank you very much.

kid51

[Util]

bubaflub's patch *does* satisfy those concerns.

My intent was (and still is) to unify the TODOs (now SKIPs) that were split up in the Perl->PIR conversion. I should not have let that intent delay the application of the patch.

In r46993, I applied t_op_inf_nan_t.patch (with whitespace trimmed to allow it to apply cleanly).

bubaflub++ kid51++

[jkeenan]

Replying to Util:

bubaflub's patch *does* satisfy those concerns. My intent was (and still is) to unify the TODOs (now SKIPs) that were split up in the Perl->PIR conversion.

Util,

Can you describe where we stand on the issues raised in this ticket? Are there specific TODOs and SKIPs that could be listed for various individuals to work on?

Thank you very much.

kid51

[jkeenan]

Replying to jkeenan:

Util, Can you describe where we stand on the issues raised in this ticket? Are there specific TODOs and SKIPs that could be listed for various individuals to work on?

Can we get an update on the status of this ticket?

Thank you very much.

kid51