Skip to content
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.

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

PLAT-156: Migrate test_fetch #1129

Merged
merged 10 commits into from
Dec 13, 2023
Merged

Conversation

ethho
Copy link
Contributor

@ethho ethho commented Dec 12, 2023

test_decimal fails because its contents, a zip object, are
exhausted by a previous test. Reproduce by seeing a pass
then a fail when running

pytest -k 'test_offset or test_decimal' tests/test_fetch.py
The following command now passes, as do all tests in this
module:

pytest -k 'test_offset or test_decimal' tests/test_fetch.py
@ethho ethho requested a review from A-Baji December 13, 2023 00:05
@ethho ethho marked this pull request as ready for review December 13, 2023 00:05
@pytest.fixture
def languages(lang) -> List:
og_contents = lang.contents
languages = og_contents.copy()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to copy, store, and restore the contents of this table because the contents would change between tests. This caused some tests such as test_fetch1_step1 to fail if other tests (that sort contents) such as test_order_by were run before it.

assert set(result[0]) == set(attrs)

def test_fetch1_step1(self, schema_any, lang, languages):
assert (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this assertion to check if upstream tests like test_offset_by changed the contents and broke this test.

@@ -307,7 +307,7 @@ class DecimalPrimaryKey(dj.Lookup):
definition = """
id : decimal(4,3)
"""
contents = zip((0.1, 0.25, 3.99))
contents = list(zip((0.1, 0.25, 3.99)))
Copy link
Contributor Author

@ethho ethho Dec 13, 2023

Choose a reason for hiding this comment

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

Strangely, the logic in test_fetch::test_offset would exhaust this zip object, causing it to be empty when test_fetch::test_decimal was run, causing the latter test to fail.

It's possible that ORDER BY on the upstream table causes DecimalPrimaryKey to also sort (and therefore exhaust the zip object):

https://github.com/ethho/datajoint-python/blob/761c1663f4a2be45e521e84f3d33ef79b34b3ba4/tests/test_fetch.py#L205-L207

    def test_offset(self, schema_any, lang, languages):
        """Tests offset"""
        cur = lang.fetch(limit=4, offset=1, order_by=["language", "name DESC"])

Whatever the cause, changing the contents from a zip to a list prevents these tests from interfering with each other.

Copy link
Member

Choose a reason for hiding this comment

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

zip can only be used once. Once exhausted, it's empty. So if you are using the same code more than once to populate a lookup table, you need to convert into a list first.

@A-Baji A-Baji merged commit bd2690d into datajoint:dev-tests Dec 13, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants