Skip to content

[libclang/python] Properly report errors when test fails #142371

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

DeinAlptraum
Copy link
Contributor

test_cdb.py's test_create_fail captures stderr to suppress output but did not release it in case the test fails.

test_cdb.py's test_create_fail captures stderr to suppress output
but did not release it in case the test fails.
@DeinAlptraum DeinAlptraum requested a review from Endilll June 2, 2025 12:01
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:as-a-library libclang and C++ API labels Jun 2, 2025
@DeinAlptraum
Copy link
Contributor Author

This issue has been raised in #142353 (review)

@llvmbot
Copy link
Member

llvmbot commented Jun 2, 2025

@llvm/pr-subscribers-clang

Author: Jannick Kremer (DeinAlptraum)

Changes

test_cdb.py's test_create_fail captures stderr to suppress output but did not release it in case the test fails.


Full diff: https://github.com/llvm/llvm-project/pull/142371.diff

1 Files Affected:

  • (modified) clang/bindings/python/tests/cindex/test_cdb.py (+10-7)
diff --git a/clang/bindings/python/tests/cindex/test_cdb.py b/clang/bindings/python/tests/cindex/test_cdb.py
index 757a14fbaef2c..5abe56f0d65f8 100644
--- a/clang/bindings/python/tests/cindex/test_cdb.py
+++ b/clang/bindings/python/tests/cindex/test_cdb.py
@@ -21,13 +21,16 @@ def test_create_fail(self):
 
         # clang_CompilationDatabase_fromDirectory calls fprintf(stderr, ...)
         # Suppress its output.
-        stderr = os.dup(2)
-        with open(os.devnull, "wb") as null:
-            os.dup2(null.fileno(), 2)
-        with self.assertRaises(CompilationDatabaseError) as cm:
-            CompilationDatabase.fromDirectory(path)
-        os.dup2(stderr, 2)
-        os.close(stderr)
+        try:
+            stderr = os.dup(2)
+            with open(os.devnull, "wb") as null:
+                os.dup2(null.fileno(), 2)
+            with self.assertRaises(CompilationDatabaseError) as cm:
+                CompilationDatabase.fromDirectory(path)
+        # Ensures that stderr is reset even if the above code crashes
+        finally:
+            os.dup2(stderr, 2)
+            os.close(stderr)
 
         e = cm.exception
         self.assertEqual(e.cdb_error, CompilationDatabaseError.ERROR_CANNOTLOADDATABASE)

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

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

Have you reproduced the issue and checked that this fix does what's it supposed to do?

@DeinAlptraum
Copy link
Contributor Author

DeinAlptraum commented Jun 2, 2025

@Endilll
I was too lazy to reproduce the exact issue (installing 32bit library and rebuilding Clang etc.) but I've forced this with a different error. The symptom (EEEEEEE% output and no error message on testing runs) is not specific to that one issue, but occurs whenever the get_cindex_library function raises.
For my own testing, I've confirmed that raising at the start of that function causes the same output, and that after adding my change you get the proper output (including the actual error message and traceback)

(just to be clear: that doesn't fix any issues related to 32bit/64bit mixing, it just ensures this test doesn't swallow all output whenever it fails)

@DeinAlptraum DeinAlptraum merged commit c5da471 into llvm:main Jun 2, 2025
16 checks passed
@DeinAlptraum DeinAlptraum deleted the cdbtest_silent_error branch June 2, 2025 14:14
sallto pushed a commit to sallto/llvm-project that referenced this pull request Jun 3, 2025
test_cdb.py's test_create_fail captures stderr to suppress output but
did not release it in case the test fails.
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
test_cdb.py's test_create_fail captures stderr to suppress output but
did not release it in case the test fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:as-a-library libclang and C++ API clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants