Fix CI failures by updating OSError handling in test finalizers#477
Fix CI failures by updating OSError handling in test finalizers#477Harishmcw wants to merge 1 commit intobastibe:masterfrom
Conversation
|
Can you point me to documentation of this behavior? This seems awfully strange. I'm also a bit saddened that we're no longer checking for the OSError. Perhaps we should explicitly disable the check only on Windows? |
|
Thanks for the question, I investigated this further. I verified that SoundFile correctly closes the file descriptor. After However, on the newer Windows CI environment ( Minimal check I used: Output:
So the FD is definitely closed, but If you prefer, we could keep the check and disable it only on Windows. |
|
This whole situation is very silly. Since soundfile does close the file on its own, there really is no point in explicitly closing it in the finalizer. I suppose we did close it just to make sure we're not leaking a resource, and it's good practice to check that it actually was closed. So I'd say we do as you propose in your reproduction, and assert explicitly that |
|
I tried this approach, but I’m seeing different behavior when running the full test suite. In the minimal example I shared earlier, However, after updating the finalizer as suggested and running the tests, there are cases where CI run with this change: https://github.com/Harishmcw/python-soundfile/actions/runs/24632959347/job/72023393030 |
|
Thank you for testing. That's very annoying. I agree with your original assessment, then, that we can't do any better than try/ignore After that, I'd be happy to merge it. Thank you for your work on this! |
This PR fixes the CI failures on Windows builds.
The Problem
windows-2022but failing onwindows-latestbecause GitHub updated the latter to Windows Server 2025. On this newer image, closing an invalid file descriptor now fails silently instead of raising an OSError. Since the test strictly expected a crash via pytest.raises(OSError), the lack of an exception caused a test failure.The Soultion
tests/test_soundfile.pyto use atry/exceptblock. This ensures the file is closed if still open, but allows the test to pass if the OS handles the closure silently or if the library has already disposed of the resource.