bpo-32374: Enhance test_importlib.test_bad_traverse()

[vstinner]

Make test_bad_traverse() fails if the script raises a Python
exception: generic exit code 1. The test expects a crash which means
an exit code different than 0 and 1.

https://bugs.python.org/issue32374

[vstinner]

I'm not 100% sure sure that the test is portable. Maybe on Windows or on other platforms, the exit code is 1 on a crash? But I'm optimistic :-)

[encukou]

AFAIK, C's assert/abort does not guarantee an exit code outside (0, 1) on all possible platforms.

[vstinner]

AFAIK, C's assert/abort does not guarantee an exit code outside (0, 1) on all possible platforms.

I wasn't sure neither, so I looked at existing tests.

If a process is killed by a signal like SIGSEGV or SIGBUS, subprocess.Popen.returncode is a negative number. subprocess.py:

            if _WIFSIGNALED(sts):
                self.returncode = -_WTERMSIG(sts)

We have many tests for that. For example, test_subprocess.test_run_abort() calls os.abort() and then checks self.assertEqual(-p.returncode, signal.SIGABRT).

On Windows, the exit code is usually a big number. The test just passed on AppVeyor.

So my PR expects that spec.loader.create_module() kills the process with any signal.

[encukou]

I'm not 100% sure sure that the test is portable. Maybe on Windows or on other platforms, the exit code is 1 on a crash? But I'm optimistic :-)

Even if all supported platforms pass, I'm not too comfortable relying on that. It would make porting to new platforms harder.

[vstinner]

The code calls spec.loader.create_module(spec) which does crash on Py_VISIT(m_state->integer) because m_state == NULL. test_faulthandler has an unit test on dereferencing a NULL pointer: test_faulthandler_read_null() which checks that the test is killed by a signal, SIGSEGV, SIGBUS or SIGILL. I would say that the signal number is not portable: it depends on the platform. But test_faulthandler pass on all platforms supported by Python.

Even if all supported platforms pass, I'm not too comfortable relying on that. It would make porting to new platforms harder.

Oh, I'm not sure that my change is the best option. It's just a proposal :-)

[encukou]

I think it's reasonable for faulthandler to be a bit platform-specific, so a new platform might have to change/disable faulthandler tests (or faulthandler itself).
But here, the competing PR depends (AFAIK) only on C standard behavior, so I prefer that one. Thanks for the discussion though!