From f1f4c8c10481ab643a497876dfaed713f54c9479 Mon Sep 17 00:00:00 2001
From: Anthony Shaw <anthonyshaw@apache.org>
Date: Tue, 27 Mar 2018 17:38:17 +1100
Subject: [PATCH] updates for code review recommendations

---
 _pytest/debugging.py |  2 +-
 testing/test_pdb.py  | 43 ++++++++++++++++++++++---------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/_pytest/debugging.py b/_pytest/debugging.py
index 68663490d..7b91fdf61 100644
--- a/_pytest/debugging.py
+++ b/_pytest/debugging.py
@@ -36,7 +36,7 @@ def pytest_configure(config):
 
     # Use custom Pdb class set_trace instead of default Pdb on breakpoint() call
     if SUPPORTS_BREAKPOINT_BUILTIN:
-        _environ_pythonbreakpoint = getattr(os.environ, 'PYTHONBREAKPOINT', '')
+        _environ_pythonbreakpoint = os.environ.get('PYTHONBREAKPOINT', '')
         if _environ_pythonbreakpoint == '':
             sys.breakpointhook = pytestPDB.set_trace
 
diff --git a/testing/test_pdb.py b/testing/test_pdb.py
index 882e4fda2..4d768585c 100644
--- a/testing/test_pdb.py
+++ b/testing/test_pdb.py
@@ -9,7 +9,7 @@ from _pytest.debugging import (SUPPORTS_BREAKPOINT_BUILTIN, pytestPDB,
 import pytest
 
 
-_ENVIRON_PYTHONBREAKPOINT = getattr(os.environ, 'PYTHONBREAKPOINT', '')
+_ENVIRON_PYTHONBREAKPOINT = os.environ.get('PYTHONBREAKPOINT', '')
 
 
 def runpdb_and_get_report(testdir, source):
@@ -59,7 +59,8 @@ def custom_debugger_hook():
             called.append("set_trace")
 
     _pytest._CustomDebugger = _CustomDebugger
-    return called
+    yield called
+    del _pytest._CustomDebugger
 
 
 class TestPDB(object):
@@ -492,35 +493,39 @@ class TestDebuggingBreakpoints(object):
         Test that sys.breakpointhook is set to the custom Pdb class once configured, test that
         hook is reset to system value once pytest has been unconfigured
         """
-        config = testdir.parseconfig()
+        testdir.makeconftest("""
+            from pytest import hookimpl
+            from _pytest.debugging import pytestPDB
 
-        pytest_configure(config)
-        assert sys.breakpointhook == pytestPDB.set_trace
+            @hookimpl(hookwrapper=True)
+            def pytest_configure(config):
+                yield
+                assert sys.breakpointhook is pytestPDB.set_trace
+
+            @hookimpl(hookwrapper=True)
+            def pytest_unconfigure(config):
+                yield
+                assert sys.breakpointhook is sys.__breakpoint__
 
-        p1 = testdir.makepyfile("""
-        def test_nothing():
-            a = 0
-            assert a == 0
         """)
-        result = testdir.runpytest_inprocess("", p1)
-        result.stdout.fnmatch_lines([
-            "* passed*",
-        ])
-        assert sys.breakpointhook != pytestPDB.set_trace
+        testdir.makepyfile("""
+            def test_nothing(): pass
+        """)
 
+    @pytest.mark.parametrize('args', [('--pdb',), ()])
     @pytest.mark.skipif(not SUPPORTS_BREAKPOINT_BUILTIN, reason="Requires breakpoint() builtin")
-    def test_sys_breakpointhook_configure_and_unconfigure_with_pdb_flag(self, testdir):
+    def test_sys_breakpointhook_configure_and_unconfigure_with_pdb_flag(self, testdir, args):
         config = testdir.parseconfig()
 
         pytest_configure(config)
         assert sys.breakpointhook == pytestPDB.set_trace
 
-        p1 = testdir.makepyfile("""
+        testdir.makepyfile("""
         def test_nothing():
             a = 0
             assert a == 0
         """)
-        result = testdir.runpytest_inprocess("--pdb", p1)
+        result = testdir.runpytest_inprocess(*args)
         result.stdout.fnmatch_lines([
             "*1 passed*",
         ])
@@ -558,10 +563,6 @@ class TestDebuggingBreakpoints(object):
 
     @pytest.mark.skipif(not SUPPORTS_BREAKPOINT_BUILTIN, reason="Requires breakpoint() builtin")
     def test_pdb_not_altered(self, testdir):
-        """
-        Test that calling PDB set_trace() is not affected
-        when PYTHONBREAKPOINT=0
-        """
         p1 = testdir.makepyfile("""
             import pdb
             def test_1():