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

Don't swallow gtest failures when running tests in LLDB #4552

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

SeanTAllen
Copy link
Member

In CI, we run our various tests in LLDB as a debugger. LLDB when run in batch mode like we are doing, always returns 0 as the exit code. It doesn't return the exit code from the program being run. The LLDB documentation states this is because more than 1 process can be being debugged so the concept that GDB has for returning the error code is more problematic with LLDB.

The result of this is that non-exception errors in CI are not being flagged because LLDB is eating the exit code from gtest when a test fails. gest offers the --gtest_throw_on_failure option that turns test failures into exceptions.

Those exceptions will be caught when we are using LLDB and reported correctly in CI.

Long term, we have discussed adding an LLDB module with python to get us the "return exit code". There is discussion on Stack Overflow of this approach at https://stackoverflow.com/questions/56456724/quit-lldb-with-exit-code-of-process/76741573#76741573

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Nov 27, 2024
@SeanTAllen
Copy link
Member Author

This should fail CI as there is a test failure that currently isn't being caught. Once we verify that it is correctly failing, we can rebase against main once the failing test has been fixed.

@dipinhora
Copy link
Contributor

@SeanTAllen i would assume we only want --gtest_throw_on_failure enabled if lldb is being used since it is likely to be unexpected if someone is running the tests locally without lldb?

@SeanTAllen
Copy link
Member Author

SeanTAllen commented Nov 27, 2024

@dipinhora will this result in an early test suite exit? if no, then, does it matter if it isn't scoped to LLDB?

@dipinhora
Copy link
Contributor

@dipinhora will this result in an early test suite exit? if no, then, does it matter if it isn't scoped to LLDB?

i believe it would be an early test exit since it would be an unhandled c++ exception for every failure but i'm not 100% sure.. in theory this should be a quick thing to confirm since we have a reproducible failure..

@SeanTAllen
Copy link
Member Author

@dipinhora i'll test now.

@dipinhora
Copy link
Contributor

@SeanTAllen yep, seems like an early exit/abort when --gtest_throw_on_failure is used:

root@5f999c466ce7:/workspaces/ponyc# ./build/debug-runtimestats/libponyrt.tests --gtest_filter='*Heap*:Pool.Fifo'                         
Note: Google Test filter = *Heap*:Pool.Fifo
[==========] Running 2 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 1 test from Heap
[ RUN      ] Heap.Init
/workspaces/ponyc/test/libponyrt/mem/heap.cc:29: Failure
Expected equality of these values:
  (size_t)(128 + 48)
    Which is: 176
  actor->actorstats.heap_mem_used
    Which is: 160
[  FAILED  ] Heap.Init (0 ms)
[----------] 1 test from Heap (0 ms total)

[----------] 1 test from Pool
[ RUN      ] Pool.Fifo
[       OK ] Pool.Fifo (0 ms)
[----------] 1 test from Pool (0 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 2 test suites ran. (0 ms total)
[  PASSED  ] 1 test.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Heap.Init

 1 FAILED TEST
root@5f999c466ce7:/workspaces/ponyc# ./build/debug-runtimestats/libponyrt.tests --gtest_filter='*Heap*:Pool.Fifo' --gtest_throw_on_failure
Note: Google Test filter = *Heap*:Pool.Fifo
[==========] Running 2 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 1 test from Heap
[ RUN      ] Heap.Init
/workspaces/ponyc/test/libponyrt/mem/heap.cc:29: Failure
Expected equality of these values:
  (size_t)(128 + 48)
    Which is: 176
  actor->actorstats.heap_mem_used
    Which is: 160
terminate called after throwing an instance of 'testing::internal::GoogleTestFailureException'
  what():  /workspaces/ponyc/test/libponyrt/mem/heap.cc:29: Failure
Expected equality of these values:
  (size_t)(128 + 48)
    Which is: 176
  actor->actorstats.heap_mem_used
    Which is: 160
Aborted
root@5f999c466ce7:/workspaces/ponyc# 

In CI, we run our various tests in LLDB as a debugger. LLDB when run
in batch mode like we are doing, always returns 0 as the exit code. It
doesn't return the exit code from the program being run. The LLDB documentation
states this is because more than 1 process can be being debugged so the
concept that GDB has for returning the error code is more problematic with
LLDB.

The result of this is that non-exception errors in CI are not being flagged
because LLDB is eating the exit code from gtest when a test fails. gest offers
the `--gtest_throw_on_failure` option that turns test failures into exceptions.

Those exceptions will be caught when we are using LLDB and reported correctly
in CI.

Long term, we have discussed adding an LLDB module with python to get us the
"return exit code". There is discussion on Stack Overflow of this approach at
https://stackoverflow.com/questions/56456724/quit-lldb-with-exit-code-of-process/76741573#76741573
@SeanTAllen SeanTAllen force-pushed the dont-swallow-gtest-errors-with-lldb branch from e16e2d1 to 2865574 Compare November 27, 2024 15:18
@SeanTAllen
Copy link
Member Author

I verified when doing this that full program tests correctly fail when run under lldb.

@SeanTAllen
Copy link
Member Author

@dipinhora updated

@dipinhora
Copy link
Contributor

This should fail CI as there is a test failure that currently isn't being caught. Once we verify that it is correctly failing, we can rebase against main once the failing test has been fixed.

#4553 has been opened to fix the failing test..

@SeanTAllen SeanTAllen merged commit a659d49 into main Nov 27, 2024
23 of 24 checks passed
@SeanTAllen SeanTAllen deleted the dont-swallow-gtest-errors-with-lldb branch November 27, 2024 17:11
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Nov 27, 2024
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

Successfully merging this pull request may close these issues.

3 participants