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

improve sub case text references #406

Open
Trass3r opened this issue Jul 28, 2020 · 3 comments
Open

improve sub case text references #406

Trass3r opened this issue Jul 28, 2020 · 3 comments

Comments

@Trass3r
Copy link

Trass3r commented Jul 28, 2020

Extracted from #321 (comment):

The message could be improved:

DEEPEST SUBCASE STACK REACHED (DIFFERENT FROM THE CURRENT ONE):

  "3"

I have no clue what the "CURRENT ONE" is and how it's different. Looks like implementation details to me.
Should be something simple like

TEST CASE:  non-literal type ("3")

Nested cases could be printed like ("3" / "4" / ...) I guess.

@Trass3r Trass3r changed the title improve sub cases text references improve sub case text references Jul 28, 2020
@onqtam
Copy link
Member

onqtam commented Sep 7, 2020

The idea behind the "current" is that we might have popped a subcase or two compared to the deepest stack we might have encountered in the current entrance of this test case. Example:

TEST_CASE("lots of nested subcases") {
    SUBCASE("1") {
        SUBCASE("2") {}
        CHECK(false);
    }
}

when we hit the assert the deepest stack is 1/2 but the current stack is just 1, so the output is this:

TEST CASE:  lots of nested subcases
  1

DEEPEST SUBCASE STACK REACHED (DIFFERENT FROM THE CURRENT ONE):
  1
  2

a.cpp:7: ERROR: CHECK( false ) is NOT correct!
  values: CHECK( false )

and both the current and the deepest subcase stacks are printed, but I guess the current output is confusing.

One other consideration behind the separate rows for each subcase is BDD-style test cases - they are effectively subcases but with a prefix (either Given: or When: or... you get the point), and for them it also makes sense to use separate rows.

Also if the names of subcases are too long then printing everything on a single line will not be readable/optimal either.

However, I agree with you that the current output is weird/confusing/ugly and for short subcase names I would totally prefer your proposal. Perhaps the order of the current/deepest should be switched (so that the deepest is printed first and the current is printed after that - closer to the actual error). Perhaps we could make a new dedicated macro for BDD-style subcases which has an additional flag to distinguish it from the normal ones, so that they are printed differently...

@Trass3r
Copy link
Author

Trass3r commented Sep 7, 2020

Ah ok I used that pattern in value-parameterized tests.
The subcase has technically gone out of scope but since it assigned something to a variable we need that deepest stack.
Maybe one could combine the two lists since they should have the same prefix, just displaying the "tail" differently, e.g. colored if we are in a terminal. Or display them as a tree.

@onqtam
Copy link
Member

onqtam commented Sep 7, 2020

Displaying as a tree seems like the right way to go indeed, and we could display the deepest nodes of the tree after a row saying deeper subcases reached before the current error: as you suggested. I think it's fine to break/improve the output of subcases & BDD-style test cases.

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