Enhancing Unit Testing by Adding sqldb Fixture #1270

jieguangzhou · 2023-11-10T08:42:14Z

Description

#673

Related Issues

Checklist

Is this code covered by new or existing unit tests or integration tests?
Did you run make test successfully?
Do new classes, functions, methods and parameters all have docstrings?
Were existing docstrings updated, if necessary?
Was external documentation updated, if necessary?

Additional Notes or Comments

superduperdb/backends/mongodb/metadata.py

jieguangzhou · 2023-11-13T08:44:12Z

superduperdb/backends/sqlalchemy/metadata.py

+    children = relationship(
        "Component",
        secondary=ParentChildAssociation.__table__,
        primaryjoin=id == ParentChildAssociation.parent_id,
        secondaryjoin=id == ParentChildAssociation.child_id,
-        backref="children",
+        backref="parents",
        cascade="all, delete",
    )


Got parents and children reversed

jieguangzhou · 2023-11-13T08:45:06Z

superduperdb/backends/sqlalchemy/metadata.py

                )
                .all()
            )
-            return sum([c.parents for c in components], [])
+            parents = [a.parent_id for a in assocations]


return unique_id, keep the same logic as MongoDBMetaDataStore

jieguangzhou · 2023-11-13T08:45:49Z

superduperdb/backends/sqlalchemy/metadata.py

@@ -251,7 +255,7 @@ def _replace_object(self, info, identifier, type_id, version):
                Component.type_id == type_id,
                Component.identifier == identifier,
                Component.version == version,
-            ).update({'dict': info})
+            ).update(info)


Fix error structure

jieguangzhou · 2023-11-13T08:46:49Z

superduperdb/backends/sqlalchemy/metadata.py

+                identifiers = [c.identifier for c in session.query(Component).all()]
+        identifiers = sorted(set(identifiers), key=lambda x: identifiers.index(x))
+        return identifiers


Deduplicate identifiers, keep the same logic with MongoDBMetaDataStore

codecov-commenter · 2023-11-27T07:36:28Z

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (34830a7) 80.33% compared to head (52617a4) 80.12%.
Report is 135 commits behind head on main.

Files	Patch %	Lines
superduperdb/backends/base/compute.py	68.96%	9 Missing ⚠️
superduperdb/backends/base/artifact.py	80.00%	1 Missing ⚠️
superduperdb/backends/base/data_backend.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1270      +/-   ##
==========================================
- Coverage   80.33%   80.12%   -0.22%     
==========================================
  Files          95      103       +8     
  Lines        6602     7341     +739     
==========================================
+ Hits         5304     5882     +578     
- Misses       1298     1459     +161

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jieguangzhou · 2023-11-28T15:28:56Z

superduperdb/backends/sqlalchemy/metadata.py

    args = Column(JSON)
    kwargs = Column(JSON)
-    method_name = Column(String)
+    method_name = Column(String(DEFAULT_LENGTH))
    stdout = Column(JSON)
    stderr = Column(JSON)


For databases such as MySQL, we need to supplement the size of this String. DEFAULT_LENGTH = 255

jieguangzhou · 2023-11-29T03:40:10Z

test/unittest/base/test_documents.py

+@pytest.mark.parametrize("db", [DBConfig.mongodb, DBConfig.sqldb], indirect=True)
+def test_only_uri(db):


Test a use case using fixtures with MongoDB and SQL databases.

jieguangzhou · 2023-11-29T03:42:27Z

test/conftest.py

+def db(request, monkeypatch) -> Iterator[Datalayer]:
+    # TODO: Use pre-defined config instead of dict here
+    param = request.param if hasattr(request, 'param') else DBConfig.mongodb
+    if isinstance(param, dict):
+        # e.g. @pytest.mark.parametrize("db", [DBConfig.sqldb], indirect=True)
+        setup_config = param.copy()
+    elif isinstance(param, tuple):
+        # e.g. @pytest.mark.parametrize("db", [(DBConfig.sqldb, {'n_data': 10})], indirect=True) # noqa: E501
+        setup_config = param[0].copy()
+        setup_config.update(param[1] or {})
+    else:
+        raise ValueError(f'Unsupported param type: {type(param)}')
+
+    monkeypatch.setattr(CFG, 'data_backend', setup_config['data_backend'])
+
+    db = create_db(CFG, **setup_config)
+    yield db


Configure the db fixture in test cases using the predefined DBConfig.

Standardize fixture creation using configuration settings, with support for MongoDB and SQL databases.

Then, every test case can use the same fixture, yet they can still have different setup behaviors.

DBConfig.mongodb_empty

DBConfig.sqldb_empty

DBConfig.mongodb (include: encoder, data, model, vector_index)

DBConfig.sqldb (include: encoder, data, model, vector_index)

The above four are the most commonly used configurations. If there are more frequently used database preset configurations, new configurations can also be preset, as follows:

DBConfig.mongodb_no_vector_index (include: encoder, data, model)

DBConfig.sqldb_no_vector_index (include: encoder, data, model)

DBConfig.mongodb_data (include: encoder, data)

DBConfig.sqldb_data (include: encoder, data)

For example:

default setup behavior with using mongodb

def test_delete_many(db): ...

empty mongodb

@pytest.mark.parametrize("db", [DBConfig.mongodb_empty], indirect=True) def test_execute_insert_and_find(db): ...

empty mongodb and empty sqldb

@pytest.mark.parametrize( "db", [DBConfig.mongodb_empty, DBConfig.sqldb_empty], indirect=True ) def test_add_version(db): ....

do not add vector_index and add 500 data points

pytest.mark.parametrize( "db", [ (DBConfig.mongodb_no_vector_index, {'n_data': 500}), (DBConfig.sqldb_no_vector_index, {'n_data': 500}), ], indirect=True, ) def test_dataset(db): ...

or use config param to do this

pytest.mark.parametrize( "db", [ (DBConfig.mongodb, {'add_vector_index': False, 'n_data': 500}), (DBConfig.sqldb, {'add_vector_index': False, 'n_data': 500}), ], indirect=True, ) def test_dataset(db): ...

do not add vector_index and add default size (5) data points

pytest.mark.parametrize( "db", [ DBConfig.mongodb_no_vector_index, DBConfig.sqldb_no_vector_index, ], indirect=True, ) def test_dataset(db): ...

This is an optimization I found while working, which involves combining the MongoDB and SQL DB fixtures. What do you think, should we do this? Or should we keep the fixtures separate? If we separate them, it might be a bit more troublesome to use different fixtures for a single test case.

WDYT? @thejumpman2323 @blythed

https://github.com/jieguangzhou/superduperdb/blob/unittests/sqllite/test/db_config.py

test/unittest/component/test_serialization.py

test/unittest/ext/test_transformers.py

blythed

Just one comment and then we can merge.

superduperdb/backends/mongodb/metadata.py

…rt for MongoDB and SQL databases.

jieguangzhou mentioned this pull request Nov 10, 2023

Modular tests: Modularize over underlying datastore in test suite #673

Closed

jieguangzhou force-pushed the unittests/sqllite branch from 544b512 to 2e1c1b6 Compare November 13, 2023 07:03

jieguangzhou commented Nov 13, 2023

View reviewed changes

superduperdb/backends/mongodb/metadata.py Outdated Show resolved Hide resolved

jieguangzhou commented Nov 13, 2023

View reviewed changes

jieguangzhou force-pushed the unittests/sqllite branch 2 times, most recently from 0211178 to 0748303 Compare November 13, 2023 11:31

blythed force-pushed the main branch from 47e3bdf to bdc31a1 Compare November 14, 2023 08:30

jieguangzhou force-pushed the unittests/sqllite branch 2 times, most recently from 6f52763 to e8b91df Compare November 27, 2023 07:33

jieguangzhou self-assigned this Nov 27, 2023

fazlulkarimweb added the 🛠️ task label Nov 27, 2023

fazlulkarimweb changed the title ~~Add sqldb fixture in UT~~ Enhancing Unit Testing by Adding sqldb Fixture Nov 27, 2023

jieguangzhou force-pushed the unittests/sqllite branch 2 times, most recently from 5b881d5 to fa9a02d Compare November 28, 2023 15:21

jieguangzhou commented Nov 28, 2023

View reviewed changes

jieguangzhou linked an issue Nov 28, 2023 that may be closed by this pull request

Modular tests: Modularize over underlying datastore in test suite #673

Closed

jieguangzhou force-pushed the unittests/sqllite branch 2 times, most recently from 3aa4fe7 to ee25d60 Compare November 29, 2023 03:39

jieguangzhou commented Nov 29, 2023

View reviewed changes

jieguangzhou force-pushed the unittests/sqllite branch from ee25d60 to 5a23976 Compare November 29, 2023 03:46

jieguangzhou marked this pull request as ready for review November 29, 2023 03:48

jieguangzhou force-pushed the unittests/sqllite branch from 5a23976 to 070193c Compare November 29, 2023 03:56

jieguangzhou requested review from thejumpman2323 and blythed November 29, 2023 03:58

jieguangzhou force-pushed the unittests/sqllite branch from 070193c to ee83876 Compare November 29, 2023 04:02

blythed reviewed Nov 29, 2023

View reviewed changes

test/unittest/component/test_serialization.py Outdated Show resolved Hide resolved

blythed reviewed Nov 29, 2023

View reviewed changes

test/unittest/ext/test_transformers.py Outdated Show resolved Hide resolved

blythed approved these changes Nov 29, 2023

View reviewed changes

blythed requested changes Nov 29, 2023

View reviewed changes

superduperdb/backends/mongodb/metadata.py Outdated Show resolved Hide resolved

superduperdb/backends/mongodb/metadata.py Outdated Show resolved Hide resolved

jieguangzhou force-pushed the unittests/sqllite branch from ee83876 to 8d8a7de Compare November 29, 2023 09:13

jieguangzhou requested a review from blythed November 29, 2023 09:19

jieguangzhou force-pushed the unittests/sqllite branch from 8d8a7de to 12c0d70 Compare November 29, 2023 09:47

Fix the bug when using sqlite as metadata link

5c14ce2

jieguangzhou force-pushed the unittests/sqllite branch from 12c0d70 to 52617a4 Compare November 29, 2023 10:40

jieguangzhou added 2 commits November 29, 2023 18:40

Standardize fixture creation using configuration settings, with suppo…

a8044b1

…rt for MongoDB and SQL databases.

Configure the db fixture in test cases using the predefined DBConfig.

52617a4

blythed approved these changes Nov 29, 2023

View reviewed changes

blythed merged commit b081063 into superduper-io:main Nov 29, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancing Unit Testing by Adding sqldb Fixture #1270

Enhancing Unit Testing by Adding sqldb Fixture #1270

jieguangzhou commented Nov 10, 2023 •

edited

Loading

jieguangzhou Nov 13, 2023

jieguangzhou Nov 13, 2023

jieguangzhou Nov 13, 2023 •

edited

Loading

jieguangzhou Nov 13, 2023

codecov-commenter commented Nov 27, 2023 •

edited

Loading

jieguangzhou Nov 28, 2023

blythed Nov 29, 2023

jieguangzhou Nov 29, 2023

blythed Nov 29, 2023

jieguangzhou Nov 29, 2023 •

edited

Loading

jieguangzhou Nov 29, 2023

blythed left a comment

		@pytest.mark.parametrize("db", [DBConfig.mongodb, DBConfig.sqldb], indirect=True)
		def test_only_uri(db):

Enhancing Unit Testing by Adding sqldb Fixture #1270

Enhancing Unit Testing by Adding sqldb Fixture #1270

Conversation

jieguangzhou commented Nov 10, 2023 • edited Loading

Description

Related Issues

Checklist

Additional Notes or Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieguangzhou Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 27, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieguangzhou Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blythed left a comment

Choose a reason for hiding this comment

jieguangzhou commented Nov 10, 2023 •

edited

Loading

jieguangzhou Nov 13, 2023 •

edited

Loading

codecov-commenter commented Nov 27, 2023 •

edited

Loading

jieguangzhou Nov 29, 2023 •

edited

Loading