Skip to content

Conversation

@benhillis
Copy link
Member

While debugging a different test failure, I noticed two things:

  1. It is possible that a crashing test process could still result in a passing test (if the test binary crashed without logging an error first)
  2. There was an incorrect printf format string causing "PASS: null" to be printed instead of the test name

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes two bugs in the test logging infrastructure to improve reliability and correctness of test result reporting. The changes prevent false positives from crashed test processes and fix incorrect printf format strings that were causing test names to be displayed as "null".

Key Changes

  • Added explicit verification of test process exit codes to catch crashes that occur without prior error logging
  • Fixed incorrect printf format strings in Linux unit test logging by removing extraneous false arguments
  • Changed test assertion approach from exception-based (THROW_HR_IF) to framework-based (VERIFY_IS_TRUE) for better test failure reporting

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/windows/Common.cpp Captures exit code in variable for logging, adds VERIFY_ARE_EQUAL check on exit code, and changes THROW_HR_IF to VERIFY_IS_TRUE for test pass/fail verification
test/linux/unit_tests/unittests.c Removes incorrect false arguments from LxtLogPassed and LxtLogError calls that were causing test names to print as "null" due to printf format mismatch

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@benhillis benhillis merged commit 0f6f3c1 into master Dec 2, 2025
12 checks passed
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