Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-24505

Tests are not marked as skipped with TAP plugin

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Minor Minor
    • tap-plugin
    • None

      I issue the following tap file:

      1..77
ok 1 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/BlockRandII_fast_asm.pp
ok 2 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/UL_searcherFindPeaks_fp_fp4014.pp
ok 3 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/VDSL2Interleaver_process8x16ICWs_NSB_big.pp
ok 4 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/abi-clobber.asm
ok 5 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/abi-clobber1.asm
ok 6 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/abi1.asm
ok 7 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/abi2.asm
ok 8 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/abi3.asm
ok 9 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/alloc1.asm
ok 10 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/branches1.asm
ok 11 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/branches2.asm
ok 12 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/crc_output_fp_fp4014.pp
ok 13 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/dotprod.asm
ok 14 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/empty-use.asm
ok 15 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/ilv4x16_fp4014.pp
ok 16 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/infinite-recursion1.asm
ok 17 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/infinite-recursion2.asm
ok 18 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/invalid-pair1.asm
ok 19 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/invalid-pair2.asm
ok 20 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/jump-write-conflict.asm
ok 21 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/ldpl.pp
not ok 22 - [ASM] /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/memset64_fp4014.vs # SKIP
ok 23 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/memset64_fp4014_parse-fail.vs
ok 24 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/msr.vs
ok 25 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/pair-unused.asm
ok 26 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/scramble-crc-encode_fp_fp4014.pp
ok 27 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/sieve_fp2012_fp4014.pp
ok 28 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/simple.asm
ok 29 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/td_deconstloop_dcache.pp
ok 30 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/td_deconstloop_dmem.pp
ok 31 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-1.asm
ok 32 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-2.asm
ok 33 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-3.asm
ok 34 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-4.asm
ok 35 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-5.asm
ok 36 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-6.asm
ok 37 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-7.asm
ok 38 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tempreg-8.asm
ok 39 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-callret1.asm
ok 40 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-callret2.asm
ok 41 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-label1.asm
ok 42 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-label2.asm
ok 43 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-p0.asm
ok 44 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-reg-flavours.asm
ok 45 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-set1.asm
ok 46 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/test-use_equ-flavours.asm
ok 47 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/ticked-pair.asm
ok 48 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/ticked-pair1.asm
ok 49 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/ticked.asm
ok 50 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3273.asm
ok 51 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3277.asm
ok 52 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3278.asm
ok 53 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3280.asm
ok 54 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3281.asm
ok 55 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3288.asm
ok 56 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3292.asm
ok 57 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3292_1.asm
ok 58 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3300.asm
ok 59 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3300_1.asm
ok 60 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3307.asm
ok 61 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3310.asm
ok 62 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3316.asm
ok 63 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3316_1.asm
not ok 64 - [RALL] /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3318.asm # SKIP
ok 65 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3321.asm
ok 66 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3321_1.pp
ok 67 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/tlfp-3321_2.pp
ok 68 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/unused-conflict.asm
ok 69 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/updates1.asm
ok 70 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/vlr-ticked.asm
ok 71 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/vlr1.asm
ok 72 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/vlr2.asm
ok 73 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/vlr_ticked-updated.asm
ok 74 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/writes-lives.asm
ok 75 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/xFFTh_fp_fp4014.pp
ok 76 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/xFFTh_radix4_fp4014.pp
ok 77 - /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/xFFTw32_fp_fp4014.pp

      Tests 22 and 64 should be marked as skipped but they are not. They just show up as failed.

          [JENKINS-24505] Tests are not marked as skipped with TAP plugin

          erik aronesty added a comment -

          This is a non-issue. The proper output for SKIP is "ok", not "not ok". See: http://testanything.org/tap-specification.html

          ok 22 - [ASM] /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/memset64_fp4014.vs # SKIP

          erik aronesty added a comment - This is a non-issue. The proper output for SKIP is "ok", not "not ok". See: http://testanything.org/tap-specification.html ok 22 - [ASM] /projects/firepath_tools/jenkins/jobs/rall/workspace/rallng/tests/asm/fp4014/memset64_fp4014.vs # SKIP

          erik aronesty added a comment -

          This is a non-issue. The #skip syntax should be following an "ok" not a "not ok". See: http://testanything.org/tap-specification.html

          erik aronesty added a comment - This is a non-issue. The #skip syntax should be following an "ok" not a "not ok". See: http://testanything.org/tap-specification.html

          Bruno P. Kinoshita added a comment -

          You are right Erik. Thanks for spotting and updating the issue.

          Bruno P. Kinoshita added a comment - You are right Erik. Thanks for spotting and updating the issue.

          Bruno P. Kinoshita added a comment -

          Released in 1.24

          Bruno P. Kinoshita added a comment - Released in 1.24

          James Howe added a comment -

          But you didn't actually change anything right? Why is this on the release notes?

          James Howe added a comment - But you didn't actually change anything right? Why is this on the release notes?

          Bruno P. Kinoshita added a comment -

          This is more for cleaning and organizing the issues. Normally the issues are reported, a developer here will check and move to another status, and when I cut the release I move all the issues marked as resolved to closed, and create the release notes.

          ps: in cases where issues are marked as not a issue, won't fix, cannot reproduce, having the release number helps me to know which version was used while investigating the problem - though I could add a comment somewhere when it is not included by the issue reporter

          Bruno P. Kinoshita added a comment - This is more for cleaning and organizing the issues. Normally the issues are reported, a developer here will check and move to another status, and when I cut the release I move all the issues marked as resolved to closed, and create the release notes. ps: in cases where issues are marked as not a issue, won't fix, cannot reproduce, having the release number helps me to know which version was used while investigating the problem - though I could add a comment somewhere when it is not included by the issue reporter

          Mathieu Clabaut added a comment -

          Hello,

          May I ask to reopen this issue for several reasons.

          1. Strictly speaking the TAP specification says nothing about the test being marked ok or not ok:

            If the directive starts with # SKIP, the test is counted as having been skipped. If the whole test file succeeds, the count of skipped tests is included in the generated output. The harness should report the text after # SKIP\S*\s+ as a reason for skipping.

            The grey box is only an example, and do not preclude the user to put other reasons than the one show, or other test result.
            Note that I'm not the only one to understand this (see https://github.com/scottcorgan/tap-out/issues/16 for example)

          2. But most importantly, it seems dangerous to me to put a test ok by default when skipped. If for a reason or another, there is a bug in the script that prevent to output the SKIP comment (case just met this afternoon), then the test is marked as ok, and no one will notice that something is wrong.

          Mathieu Clabaut added a comment - Hello, May I ask to reopen this issue for several reasons. Strictly speaking the TAP specification says nothing about the test being marked ok or not ok : If the directive starts with # SKIP, the test is counted as having been skipped. If the whole test file succeeds, the count of skipped tests is included in the generated output. The harness should report the text after # SKIP\S*\s+ as a reason for skipping. The grey box is only an example, and do not preclude the user to put other reasons than the one show, or other test result. Note that I'm not the only one to understand this (see https://github.com/scottcorgan/tap-out/issues/16 for example) But most importantly, it seems dangerous to me to put a test ok by default when skipped. If for a reason or another, there is a bug in the script that prevent to output the SKIP comment (case just met this afternoon), then the test is marked as ok , and no one will notice that something is wrong.

          Mathieu Clabaut added a comment -

          See my last comment about the reason of reopening.

          I hope it is not considered has being rude to reopen.

          Mathieu Clabaut added a comment - See my last comment about the reason of reopening. I hope it is not considered has being rude to reopen.

          James Howe added a comment -

          Jenkins supports three states PASS, FAIL, SKIP.
          TAP output should be parsed into those boxes.
          Then, Jenkins allows you to decide whether to mark the build Successful, Failed or Unstable based on the test state totals.
          This gives greatest utility to users, and supports both of our use cases.

          If you really cannot deal with it, raise a separate issue allowing the parsing of skipped tests to be configurable, but don't change the default.

          James Howe added a comment - Jenkins supports three states PASS, FAIL, SKIP. TAP output should be parsed into those boxes. Then, Jenkins allows you to decide whether to mark the build Successful, Failed or Unstable based on the test state totals. This gives greatest utility to users, and supports both of our use cases. If you really cannot deal with it, raise a separate issue allowing the parsing of skipped tests to be configurable, but don't change the default.

          Bruno P. Kinoshita added a comment -

          matclab reopening issues is always fine. Thanks for commenting here and sharing your view on the issue.

          jameshowe agreed.

          I believe we decided to stick with #SKIP tests one way that won't support all cases, as Mathieu's. As James correctly pointed, keeping backward compatibility, and trying to be concise is important.

          So here's a proposed solution:

          1. add an option under Advanced "Skip tests will be marked as" which has a combo box
          2. the combo box will have the values "Failure", "Success", "Skipped", "TAP Test Result status", where the last one means, 'whatever is defined in the test result'. So ok # SKIP will result in Success, not ok # SKIP will result in Failure
          3. the default value for this combo box will be "TAP Test Result status", which seems to be the current behavior
          4. a help file will be added, as well as unit tests

          Any objections?

          Bruno P. Kinoshita added a comment - matclab reopening issues is always fine. Thanks for commenting here and sharing your view on the issue. jameshowe agreed. I believe we decided to stick with #SKIP tests one way that won't support all cases, as Mathieu's. As James correctly pointed, keeping backward compatibility, and trying to be concise is important. So here's a proposed solution: 1. add an option under Advanced "Skip tests will be marked as" which has a combo box 2. the combo box will have the values "Failure", "Success", "Skipped", "TAP Test Result status", where the last one means, 'whatever is defined in the test result'. So ok # SKIP will result in Success, not ok # SKIP will result in Failure 3. the default value for this combo box will be "TAP Test Result status", which seems to be the current behavior 4. a help file will be added, as well as unit tests Any objections?

            kinow Bruno P. Kinoshita
            pmatos Paulo Matos
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated: