DatabaseAPI20Test.test_nextset is defined twice; the real implementation is dead code

[mathewOracle]

DatabaseAPI20Test.test_nextset is defined twice in a row in tests/dbapi20.py:

def test_nextset(self):
    con = self._connect()
    try:
        cur = con.cursor()
        if not hasattr(cur,'nextset'):
            return
        try:
            self.executeDDL1(cur)
            sql=self._populate()
            for sql in self._populate():
                cur.execute(sql)
            self.help_nextset_setUp(cur)
            cur.callproc('deleteme')
            numberofrows=cur.fetchone()
            assert numberofrows[0]== len(self.samples)
            assert cur.nextset()
            names=cur.fetchall()
            assert len(names) == len(self.samples)
            s=cur.nextset()
            assert s is None, 'No more return sets, should return None'
        finally:
            self.help_nextset_tearDown(cur)
    finally:
        con.close()

def test_nextset(self):
    raise NotImplementedError('Drivers need to override this test')

(lines 734 and 763 on current main)

Since Python class bodies just execute top to bottom, the second definition replaces the first. The full ~25-line implementation above never runs for anything that inherits DatabaseAPI20Test and doesn't override test_nextset itself -- it's dead code.

It doesn't affect psycopg2's own test suite today, since Psycopg2Tests in tests/test_psycopg2_dbapi20.py overrides test_nextset itself, so the base class version (either one) never runs there. But it's genuinely broken as shared test infrastructure, and I could see this file getting reused or refactored later without that override still being in place.

I checked history -- this goes back further than the last few years of commits touching this file (which are just pyupgrade/flynt/typo passes), so it's clearly not a recent or deliberate change, just a leftover.

Not sure which direction is correct, which is why I'm filing this as an issue rather than guessing in a PR:

• Delete the second stub and keep the real implementation (makes the base class actually test nextset() for any driver that doesn't override it, but that could immediately start failing/erroring for drivers whose nextset() implementation doesn't match this test's assumptions), or
• Delete the first implementation and keep the stub (matches the stub's own stated intent that "drivers need to override this test", but throws away a real, working test body for no reason)

Happy to send a PR for whichever direction you'd prefer.

[dvarrazzo]

AFAICS in psycopg 3 we dropped the second implementation, but then also overrode the first one with a no-op, and implemented proper tests in our test suite. I don't know if that test has really ever run. If you would like to do anything with it that's no problem, but IIRC psycopg 2 doesn't even implement nextset so the utility is really limited.