gh-108558: Add a couple more sqlite Row.keys() tests

[EddInSverige]

• Fix for Add explicit test for sqlite3.Row.keys() #108558, adding additional test coverage for sqlite3.Row.keys() which currently only has one test already implicitly tested elsewhere.
• Specifically, test .keys() on an empty row, check the documented statement that keys() return the first value from the description tuple, and the (possibly trivial) fact that .keys() are equal when the Row is equal.

• Issue: Add explicit test for sqlite3.Row.keys() #108558

[erlend-aasland]

Nice! Can you split it up in multiple tests?

[erlend-aasland]

Can you narrow this down? Where is the AttributeError raised? Consider also using assertRaisesRegex.

[serhiy-storchaka]

What is the type of row?

[EddInSverige]

So .fetchone() is returning None as per the comment below,

    # Returns either a row (as created by the row_factory) or None, but
    # putting None in the return annotation causes annoying false positives.
    def fetchone(self) -> Any: ...

and the type of keys has to be a list of strings,

class Row:
    def __init__(self, __cursor: Cursor, __data: tuple[Any, ...]) -> None: ...
    def keys(self) -> list[str]: ...

and here is the C code for the keys method,

static PyObject *
pysqlite_row_keys_impl(pysqlite_Row *self)
/*[clinic end generated code: output=efe3dfb3af6edc07 input=7549a122827c5563]*/
{
    PyObject* list;
    Py_ssize_t nitems, i;

    list = PyList_New(0);
    if (!list) {
        return NULL;
    }
    nitems = PyTuple_Size(self->description);

    for (i = 0; i < nitems; i++) {
        if (PyList_Append(list, PyTuple_GET_ITEM(PyTuple_GET_ITEM(self->description, i), 0)) != 0) {
            Py_DECREF(list);
            return NULL;
        }
    }

    return list;
}

however I'm not familiar with the codebase enough to go between the C code and the Python code to find where the error is being raised :(

[erlend-aasland]

Since the query fails, no rows are returned, so fetchone returns None, so what you are really testing here is that None does not have a keys attribute :) I think we can do without this test.

[erlend-aasland]

What is the type of row?

None :) As already mentioned, we don't need this test.

[serhiy-storchaka]

If fetchone() returns not a Row, this code has no any relation to testing Row.keys().

[erlend-aasland]

See #108628 (comment), Serhiy.

[erlend-aasland]

FTR, it is also possible to do:

Suggested change

	cur.execute("select 1 as a")
	row = cur.fetchone()
	row ,= cur.execute("select 1 as a")

and:

Suggested change

	cur.execute("select 1 as a")
	row = cur.fetchone()
	[row] = cur.execute("select 1 as a")

... though they are pretty cryptic 😄

[serhiy-storchaka]

I am not sure that the new tests have any value. It is not clear what they do, why they are necessary. They just distract attention from more meaningful tests.

[serhiy-storchaka]

What is the type of row?

[erlend-aasland]

Do you object to this test as well, @serhiy-storchaka?

I think it would be simplified as such, though:

Suggested change

	def test_sqlite_row_keys_are_equal_if_rows_are_equal(self):
	# Two Row objects compare equal if they have identical column names
	# and values. Assert the keys values are the same too.
	row_1 = self.con.execute("select 1 as a, 2 as b").fetchone()
	row_2 = self.con.execute("select 1 as a, 2 as b").fetchone()
	self.assertEqual(row_1, row_2)
	self.assertEqual(row_1.keys(), row_2.keys())
	def test_sqlite_row_equality(self):
	# Two Row objects compare equal if they have identical column names
	# and values.
	row_1 = self.con.execute("select 1 as a, 2 as b").fetchone()
	row_2 = self.con.execute("select 1 as a, 2 as b").fetchone()
	self.assertEqual(row_1, row_2)

[erlend-aasland]

OTOH, this is already tested by test_sqlite_row_hash_cmp, so it is redundant.