kward / shunit2

shUnit2 is a xUnit based unit test framework for Bourne based shell scripts.
Apache License 2.0
1.59k stars 213 forks source link

Should there be a counter for skipped tests in addition to the number of skipped asserts? #160

Open JussiPekonen opened 2 years ago

JussiPekonen commented 2 years ago

The current startSkipping/endSkipping functionality is all about skipping asserts within the test cases. While that solution can be used for skipping test cases completely, the counter for the "skips" counts the number of skipped asserts instead of skipped test cases. This can lead to situations where the reported number of skipped tests does not match the number of test cases that were skipped.

For example, consider a test case

testFoo() {
  startSkipping "Let's skip these asserts"
  assertTrue "${SHUNIT_FALSE}"
  assertFalse "${SHUNIT_TRUE}"
  endSkipping
}

In the end of the generated report, there would be something like "OK/FAILED (skipped=X+2)" where X is the number of other assert skips. Should this test case be the only one to use skipping, the report could be interpreted to include 2 test cases that were skipped.

I would like to open a discussion on the topic. Do you see any benefits of having a skipping mechanism for individual test cases as well? If so, it would benefit from a separate counter that would be reported in the end of the test run. Furthermore, the current skipped counter could be re-used in the report to indicate how many asserts were skipped.

If this gets support, I can do the needed work to get this done.

williamdes commented 2 years ago

Can you give us some feedback on this one @kward ?

JussiPekonen commented 2 years ago

One thing I forgot to mention in the original issue description is that should there ever be support for jUnit test reporting (see #16 and #127), this would make it possible to report the number of skipped test correctly in that report as well.

JussiPekonen commented 2 years ago

After rethinking this a bit, it would make sense to keep the current skipped asserts implementation and refine the generated report to indicate the number of skipped asserts more clearly. That's why I updated the title of the issue.

Should the skipped tests feature be implemented, I would suggest the following changes to be made:

  1. Introduce new functions for skipping asserts, startSkippingAsserts, endSkippingAsserts, and isSkippingAsserts, that would ultimately replace the current *Skipping functions. The old functions would be kept for backwards compatibility, but they would print out a warning message about deprecation. Tests and documentation would be updated accordingly.
  2. New functions, startSkippingTests and endSkippingTests (and possibly isSkippingTests), would be added. When the start function is called, tests after that line would be skipped until the end of the file or when the end function is called, after which the tests would not skipped anymore.
  3. New counter would be introduced to count the number of skipped tests. This would be printed out in the generated report similar to failures and skipped asserts now.
JussiPekonen commented 2 years ago

I am about to get these changes done. Some polishing is still needed before I can make a PR for the changes.

The proposal I had in the previous comment had to be tweaked a little. My suggestion for the changes would be the following:

  1. Introduce new functions for skipping verifications, startSkippingAsserts, endSkippingAsserts, and isSkippingAsserts. The old functions *Skipping are kept for backwards compatibility, but those will print out a warning message about the function being deprecated.
  2. New function, skipTest is introduced. It must be called as the very first thing in the function (before any asserts and fails) and it will disable the verification calls completely. Furthermore, it will not step the counters for the total, passed, failed, and skipped asserts. This function must be called for all tests that are wished to be skipped. In addition, it will print out a warning to stderr that the test is being skipped.
  3. The generated report will print out the number of failed and skipped tests in addition to the failed and skipped asserts (that are currently printed out in the report).
JussiPekonen commented 2 years ago

Draft PR #161 created for these changes. Feel free to check it out.