Skip to content

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
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