Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 17, 2025

Problem

The test_external_crash_reporter_http test was flaky when running under kcov code coverage analysis. The issue is that kcov exits with code 0 even when the process it's monitoring crashes with a signal like SIGSEGV:

kcov: Process exited with signal 11 (SIGSEGV) at 0x7ffff7729c5c
# exit code: 0

This caused the assertion assert child.returncode to fail because 0 is falsy in Python, making the test fail intermittently when running in the kcov CI job.

Solution

Skip the test_external_crash_reporter_http test when running under kcov using the @pytest.mark.skipif decorator. The fix uses the existing is_kcov condition variable from tests/conditions.py:

@pytest.mark.skipif(is_kcov, reason="kcov exits with 0 even when the process crashes")
def test_external_crash_reporter_http(cmake, httpserver, build_args):
    ...

This approach is consistent with how other analyzer-specific behaviors are handled in the codebase (e.g., has_breakpad and has_crashpad already skip tests entirely when kcov is running).

Tests Updated

Only test_external_crash_reporter_http is skipped when running with kcov. Other inproc crash tests work correctly with kcov and do not need to be skipped.

Note: Breakpad and Crashpad tests are already completely skipped when kcov is running via the has_breakpad and has_crashpad conditions, so they don't require updates.

Fixes https://github.com/getsentry/sentry-native/actions/runs/18502219462/attempts/1 and https://github.com/getsentry/sentry-native/actions/runs/18594256344/job/53016350698

cc @jpnurmi
Fixes #1418

Original prompt

This section details on the original issue you should resolve

<issue_title>test_external_crash_reporter_http flaky</issue_title>
<issue_description>- https://github.com/getsentry/sentry-native/actions/runs/18502219462/attempts/1

cc @jpnurmi </issue_description>

Comments on the Issue (you are @copilot in this section)

@jpnurmi Hmm, we expect a non-zero exit code for a crashed process, but `kcov` exits with zero even if it prints this right above 🤨

kcov: Process exited with signal 11 (SIGSEGV) at 0x7ffff7729c5c</comment_new>

Fixes #1418


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: vaind <6349682+vaind@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix flaky behavior in external crash reporter HTTP test Fix flaky test_external_crash_reporter_http when running with kcov Oct 17, 2025
Copilot AI requested a review from vaind October 17, 2025 16:16
@vaind
Copy link
Contributor

vaind commented Oct 17, 2025

@copilot we should just skip the whole test case in kcov then

@vaind vaind changed the title Fix flaky test_external_crash_reporter_http when running with kcov test: Fix flaky test_external_crash_reporter_http when running with kcov Oct 17, 2025
… assertions

Co-authored-by: vaind <6349682+vaind@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 17, 2025

@copilot we should just skip the whole test case in kcov then

Done. Added @pytest.mark.skipif(is_kcov, ...) decorator to skip the entire test cases instead of conditional assertions. This is cleaner and consistent with how breakpad/crashpad tests handle kcov (5395065).

Copilot AI changed the title test: Fix flaky test_external_crash_reporter_http when running with kcov Fix flaky test_external_crash_reporter_http when running with kcov Oct 17, 2025
@vaind
Copy link
Contributor

vaind commented Oct 17, 2025

@copilot we only care about test_external_crash_reporter_http - the rest works even with kcov right?

Co-authored-by: vaind <6349682+vaind@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 17, 2025

@copilot we only care about test_external_crash_reporter_http - the rest works even with kcov right?

You're right - removed the skipif decorator from the other three tests. Only test_external_crash_reporter_http is skipped when running with kcov now (153f78a).

@vaind vaind changed the title Fix flaky test_external_crash_reporter_http when running with kcov test: Fix flaky test_external_crash_reporter_http when running with kcov Oct 18, 2025
@vaind vaind marked this pull request as ready for review October 19, 2025 10:37
@vaind vaind requested a review from jpnurmi October 19, 2025 10:37
Copy link
Collaborator

@jpnurmi jpnurmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@vaind vaind merged commit e041a2b into master Oct 19, 2025
41 checks passed
@vaind vaind deleted the copilot/fix-flaky-external-crash-reporter branch October 19, 2025 18:11
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.

test_external_crash_reporter_http flaky

3 participants