Skip to content

test: tweak test data for the Text type #67

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.

Sign up for GitHub

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

Merged
IlyaFaer merged 13 commits into from
May 25, 2021
Merged

test: tweak test data for the Text type #67

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ca81ad1
test: tweak test data for the Text type
May 5, 2021
9f1fc46
fix conflicts
May 11, 2021
d664cbc
Merge branch 'main' into text_nullable
May 12, 2021
2d13fd9
Merge branch 'main' into text_nullable
May 12, 2021
b9ccd4a
del excess overrides
May 12, 2021
03dd495
Merge branch 'text_nullable' of https://github.com/cloudspannerecosys…
May 12, 2021
b835455
del excess import
May 12, 2021
1d56ad7
Merge branch 'main' into text_nullable
May 18, 2021
c16d579
resolve conflicts
May 18, 2021
57a928d
Merge branch 'main' into text_nullable
AVaksman May 19, 2021
4651a81
Merge branch 'main' into text_nullable
May 25, 2021
be83d97
revert order
May 25, 2021
a253e42
Merge branch 'text_nullable' of https://github.com/cloudspannerecosys…
May 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 13 additions & 43 deletions test/test_suite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
from sqlalchemy import String
from sqlalchemy.types import Integer
from sqlalchemy.types import Numeric
from sqlalchemy.types import Text
from sqlalchemy.testing import requires

from google.api_core.datetime_helpers import DatetimeWithNanoseconds
Expand Down Expand Up @@ -781,58 +782,27 @@ def test_literal_non_ascii(self):


class TextTest(_TextTest):
def test_text_empty_strings(self, connection):
"""
SPANNER OVERRIDE:

Cloud Spanner doesn't support the tables with an empty primary key
when column has defined NOT NULL - following insertions will fail with
`400 id must not be NULL in table date_table`.
Overriding the tests and adding a manual primary key value to avoid the same
failures.
"""
text_table = self.tables.text_table

connection.execute(text_table.insert(), {"id": 1, "text_data": ""})
row = connection.execute(select([text_table.c.text_data])).first()
eq_(row, ("",))

def test_text_null_strings(self, connection):
@classmethod
def define_tables(cls, metadata):
"""
SPANNER OVERRIDE:

Cloud Spanner doesn't support the tables with an empty primary key
when column has defined NOT NULL - following insertions will fail with
`400 id must not be NULL in table date_table`.
Overriding the tests and adding a manual primary key value to avoid the same
failures.
Cloud Spanner doesn't support auto incrementing ids feature,
which is used by the original test. Overriding the test data
creation method to disable autoincrement and make id column
nullable.
"""
text_table = self.tables.text_table

connection.execute(text_table.insert(), {"id": 1, "text_data": None})
row = connection.execute(select([text_table.c.text_data])).first()
eq_(row, (None,))
Table(
"text_table",
metadata,
Column("id", Integer, primary_key=True, nullable=True),
Column("text_data", Text),
)
Copy link
Contributor Author

@IlyaFaer IlyaFaer May 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is actually testing Text type, so I suppose we can make the id column (which is Integer and not directly considered by the test) nullable and without autoincrement (which is not supported anyway). The change makes three more tests passing (there are some cleanup/teardown errors, but the tests themselves are working fine - we'll take a look at cleanups little bit later).

Безымянный

Copy link
Contributor

@HemangChothani HemangChothani May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #36 PR will cover all text type tests.

Copy link
Contributor Author

@IlyaFaer IlyaFaer May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#36 does too many overrides. You only need to change several symbols to add manual primary key, but it requires to copy the whole test source code for every failing test.

Plus to this, #36 includes some unclear changes, like this one:

Безымянный

It looks pretty much like a meaningful diverge from the original test. Instead all of it just a single test data override can be done, keeping the original tests as they were.


@pytest.mark.skip("Spanner doesn't support non-ascii characters")
def test_literal_non_ascii(self):
pass

def test_text_roundtrip(self):
"""
SPANNER OVERRIDE:

Cloud Spanner doesn't support the tables with an empty primary key
when column has defined NOT NULL - following insertions will fail with
`400 id must not be NULL in table date_table`.
Overriding the tests and adding a manual primary key value to avoid the same
failures.
"""
text_table = self.tables.text_table

config.db.execute(text_table.insert(), {"id": 1, "text_data": "some text"})
row = config.db.execute(select([text_table.c.text_data])).first()
eq_(row, ("some text",))


class NumericTest(_NumericTest):
@emits_warning(r".*does \*not\* support Decimal objects natively")
Expand Down