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
Skip decorator for tests requiring manual intervention on Windows #55941
Comments
Attached is a patch which adds skip_unless_unattended, which ideally would be used with at least test_faulthandler when running on Windows. Running the tests on a Windows desktop results in the user having to click through Windows Error Reporting dialogs in order to continue. Build slaves disable or handle WER dialogs in order to continue running without manual intervention. The patch looks in the common registry keys [0,1] to see the status of WER and skips tests when it finds that WER is enabled and would require user intervention. We may want to additionally hook this up to a regrtest command line parameter to override the registry setting. Maybe there's a better name for the decorator - I'm not stuck with what I suggested. I consider build slaves as running in "unattended" mode as opposed to a human kicking off the tests. [0] http://www.spyany.com/program/registry-disable-error-reporting.htm |
Attached is an example of how this might be used with Lib/test/test_faulthandler.py |
Ah yes, I have also issues with the Windows fault handler (the popup). I disabled this popup manually in my Windows XP box. Instead of skipping the test, I prefer to disable temporary the popup by setting the right registry key (and then restore the previous value or delete the key). faulthandler causes fatal errors, but test_capi does also test a crash (I don't know which test exactly). I always have to click on the popup to continue to execute the test suite :-( |
Ubuntu has also a fault handler: Apport. Fedora has abrt. If we should to disable the Windows fault handler, we may also disable these tools. test_faulthandler already disable another fault handler: it disables the creation of core files. prepare_subprocess() calls setrlimit(RLIMIT_CORE, (0, 0)). |
Disabling and re-enabling is another possibility, and it's probably more likely to help in the long run. Most (all?) Windows machines have error reporting enabled unless you mess with the registry manually, so the only place tests would be run with my first patch would be on build slaves. I don't currently have a Ubuntu or Fedora machine, but I could look into it. I'll write up a patch that disables/re-enables for Windows and see how it works. |
Issue bpo-12481 has been marked as duplicate: "I'm getting the dreaded "python_d.exe has stopped working" popups in test_faulthandler on Windows 7 + VisualStudioPro + "Debug|x64"." |
"Most (all?) Windows machines have error reporting enabled unless you mess with the registry manually" It is disabled on all Windows buildbots. test_capi does also test a crash (I don't know/remember which test exactly). |
For test_capi the patch in bpo-9116 works. For test_faulthandler it |
"Instead of skipping the test, I prefer to disable temporary the popup by setting the right registry key (and then restore the previous value or delete the key)." Do you mean that the user should change/restore the key or that the BTW, it would be a great help to print a warning (also in test_capi) |
Python can change temporary the registry value for the duration of the test. |
To avoid messing with system registry settings it sounds like WerRegisterRuntimeExceptionModule could also work, at least on Windows7 http://msdn.microsoft.com/en-us/library/windows/desktop1/dd408167%28v=VS.85%29.aspx There could be a dll which would do nothing when receiving the crash report for expected crashes (test_capi and test_faulthandler). WerRegisterRuntimeExceptionModule doesn't exist in the headers of the SDK that comes with VS2008 but it could be retrieved at runtime with GetProcAddress. It's more work than the registry setting but seems cleaner since it avoids changing system settings. |
That would certainly be preferable when available on Windows 7. I'll look into how we can incorporate that. Thanks for the idea! |
Here is a proof of concept patch that defines a context manager that disables the crash popups using SetErrorMode 0. I tested it only on Windows 7 (and Linux, where it's a no-op). As an example, the patch fixes the crash popup caused by test_capi. FWIW I attempted to use WerRegisterRuntimeExceptionModule too, and I was able to access it from ctypes.windll.kernel32.WerRegisterRuntimeExceptionModule on Win7, but then it wanted me to define a DLL with 3 functions, so I tried to find a simpler solution. |
Attached an updated patch:
I tested it on Windows 7 and Linux (where it's a no-op), and according to MSDN it should work even on Windows XP. If there aren't other comments I'll commit this soon. If test_capi is also crashing on 2.7/3.2, I will probably backport it there too. |
New changeset 834a451f1cdb by Ezio Melotti in branch '3.3': New changeset b87123015fb0 by Ezio Melotti in branch 'default': |
I applied this only on 3.3/3.x because I didn't manage to test on 2.7/3.2. |
Attached an untested patch against 3.2. Unless I'm missing something:
Brian, if you can confirm this I'll go ahead, backport the patch on 3.2, and null merge it on 3.3/default. |
New changeset 6ccefddc13fd by Ezio Melotti in branch '3.3': New changeset 831035bda9b7 by Ezio Melotti in branch 'default': |
New changeset c0c440dcb8dd by Ezio Melotti in branch '3.2': New changeset 89d62bc81e47 by Ezio Melotti in branch '3.3': New changeset 7e818490d297 by Ezio Melotti in branch 'default': |
GetErrorMode wasn't available on XP/2k3, so I changed the patch to use only SetErrorMode. I also backported it to 3.2, since ctypes has the same crashing test of 3.3/default. |
I just noticed that regrtest also has a --nowindows flag that uses SetErrorMode (see Lib/test/regrtest.py:490).
Do you think this flag should be removed? |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: