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

Abort execution of a subcase without halting the entire test case #820

Closed
AmaranthineCodices opened this issue Oct 11, 2023 · 1 comment
Closed
Labels
verdict/duplicate Has been discussed before or is already solved

Comments

@AmaranthineCodices
Copy link

Description

Hi! We're using doctest 2.4.9 at Luau for our unit tests. We've run into an issue with subcases that, at this point, is making them not terribly useful for us: if, when executing a subcase, the test case exits without running to completion, all (lexically) subsequent subcases are not run, regardless of nesting depth. This appears most obviously if you use a REQUIRE macro (or otherwise throw) in a test case, but it doesn't require exceptions at all; just a return will do it.

We've made liberal use of REQUIRE macros in order to halt test execution if an assertion fails. Usually this is checking something that will make follow-up CHECK macros fail in a less-obvious manner, like the size of an array before we inspect its first element. Since there doesn't appear to be a way to actually abort execution of a subcase, we don't have many cases where we can use subcases. It would be really helpful if there were some way to abort the execution of a subcase without preventing following subcases from being executed.

Steps to reproduce

This isn't a full example, but this is the kind of test case we'd like to write:

TEST_CASE("test")
{
    // Prints once.
    MESSAGE("test");

    SUBCASE("before_fail")
    {
        // Prints once, as expected.
        MESSAGE("before_fail");
    }

    SUBCASE("fails")
    {
        // Prints once.
        MESSAGE("fails");

        SUBCASE("deeper_fail")
        {
            // Prints once, as expected.
            MESSAGE("deeper_fail");
            REQUIRE(false);
            // After this point, execution halts. No further subcases are invoked.
        }

        SUBCASE("deeper_ok")
        {
            // Should execute, but doesn't.
            MESSAGE("deeper_ok");
        }
    }

    SUBCASE("should_run")
    {
        // Should execute, but doesn't.
        MESSAGE("should_run");
    }
}

If I replace the REQUIRE(false); macro with just return;, I get the same behavior.

Extra information

  • doctest version: See source here. Based on doctest 2.4.9 with compatibility patches. A diff is included below.
  • Operating System: macOS 13.5.1
  • Compiler: Apple clang version 14.0.3 (clang-1403.0.22.14.1)
  • Platform: arm64
*** doctest-upstream.h	Wed Oct 11 09:56:34 2023
--- doctest-luau.h	Wed Oct 11 09:56:14 2023
***************
*** 3139,3145 ****
--- 3139,3149 ----
  #include <unordered_set>
  #include <exception>
  #include <stdexcept>
+
+ #if !defined(DOCTEST_CONFIG_NO_POSIX_SIGNALS)
  #include <csignal>
+ #endif
+
  #include <cfloat>
  #include <cctype>
  #include <cstdint>
***************
*** 5667,5672 ****
--- 5671,5678 ----
                  std::tm timeInfo;
  #ifdef DOCTEST_PLATFORM_WINDOWS
                  gmtime_s(&timeInfo, &rawtime);
+ #elif defined(DOCTEST_CONFIG_USE_GMTIME_S)
+                 gmtime_s(&rawtime, &timeInfo);
  #else // DOCTEST_PLATFORM_WINDOWS
                  gmtime_r(&rawtime, &timeInfo);
  #endif // DOCTEST_PLATFORM_WINDOWS
@cdeln
Copy link

cdeln commented Aug 16, 2024

Duplicate of #844 (Note that his was actually filed before the other issue, but I started reviewing issues from new to old (switching now so this does not happen again).

Note that this is the intended behavior of doctest.
A simple workaround is to replace REQUIRE with CHECK + if.
I am curious why this feature is so important for you (that is, why this makes doctest "not terribly useful" for you).
If you want to discuss this please do that in the other thread and I will close this issue to make the discussion better.

@cdeln cdeln closed this as not planned Won't fix, can't repro, duplicate, stale Aug 16, 2024
@cdeln cdeln added the verdict/duplicate Has been discussed before or is already solved label Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verdict/duplicate Has been discussed before or is already solved
Projects
None yet
Development

No branches or pull requests

2 participants