Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JussiPekonen opened this issue Jun 27, 2022 · 5 comments

Comments

@JussiPekonen
Copy link

JussiPekonen commented Jun 27, 2022

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
Copy link
Collaborator

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

@JussiPekonen
Copy link
Author

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 JussiPekonen changed the title Should there be a counter for skipped tests instead of or in addition to the number of skipped asserts? Should there be a counter for skipped tests in addition to the number of skipped asserts? Jun 28, 2022
@JussiPekonen
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants