Ticket #1302 (new bug)

Opened 5 years ago

Last modified 3 years ago

PIR todo() is frequently misused

Reported by: Util Owned by:
Priority: normal Milestone:
Component: testing Version: 1.8.0
Severity: medium Keywords: test more pir todo
Cc: Util, kid51 Language:
Patch status: Platform: all

Description

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...

Attachments

t_op_inf_nan_t.patch Download (5.6 KB) - added by bubaflub 5 years ago.
patch to change todo to skip in t/op/inf_nan.t

Change History

  Changed 5 years ago by 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.

  Changed 5 years ago by 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')
    

  Changed 5 years ago by 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!
  1. Mixed usage:
    • t/oo/metamodel.t
      1. Correct via conditioning
      2. Misused via 0
  1. Hardcoded 1 - all need to have conditioning logic:
    • t/op/arithmetics.t
    • t/op/io.t
    • t/op/number.t
    • t/pmc/float.t
  1. 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.

  Changed 5 years ago by Util

  • keywords todo added

  Changed 5 years ago by bubaflub

Is this what you were looking for?

Changed 5 years ago by bubaflub

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

  Changed 5 years ago by jkeenan

  • cc kid51 added

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

Thank you very much.

kid51

follow-up: ↓ 8   Changed 5 years ago by 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++

in reply to: ↑ 7 ; follow-up: ↓ 9   Changed 4 years ago by 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

in reply to: ↑ 8   Changed 3 years ago by 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

Note: See TracTickets for help on using tickets.