aminghadersohi commented on code in PR #35018:
URL: https://github.com/apache/superset/pull/35018#discussion_r2334800565
##########
tests/unit_tests/dao/base_dao_test.py:
##########
@@ -37,6 +44,103 @@ class TestDAOWithNoneModel(BaseDAO[MockModel]):
model_cls = None
+# =============================================================================
+# Unit Tests - These tests use mocks and don't touch the database
+# =============================================================================
+
+
+def test_column_operator_enum_apply_method() -> None: # noqa: C901
+ """
+ Test that the apply method works correctly for each operator.
+ This verifies the actual SQL generation for each operator.
+ """
+ Base_test = declarative_base() # noqa: N806
+
+ class TestModel(Base_test): # type: ignore
+ __tablename__ = "test_model"
+ id = Column(Integer, primary_key=True)
+ name = Column(String(50))
+ age = Column(Integer)
+ active = Column(Boolean)
+
+ # Test each operator's apply method
+ test_cases = [
+ # (operator, column, value, expected_sql_fragment)
+ (ColumnOperatorEnum.eq, TestModel.name, "test", "name = 'test'"),
+ (ColumnOperatorEnum.ne, TestModel.name, "test", "name != 'test'"),
+ (ColumnOperatorEnum.sw, TestModel.name, "test", "name LIKE 'test%'"),
+ (ColumnOperatorEnum.ew, TestModel.name, "test", "name LIKE '%test'"),
+ (ColumnOperatorEnum.in_, TestModel.id, [1, 2, 3], "id IN (1, 2, 3)"),
+ (ColumnOperatorEnum.nin, TestModel.id, [1, 2, 3], "id NOT IN (1, 2,
3)"),
+ (ColumnOperatorEnum.gt, TestModel.age, 25, "age > 25"),
+ (ColumnOperatorEnum.gte, TestModel.age, 25, "age >= 25"),
+ (ColumnOperatorEnum.lt, TestModel.age, 25, "age < 25"),
+ (ColumnOperatorEnum.lte, TestModel.age, 25, "age <= 25"),
+ (ColumnOperatorEnum.like, TestModel.name, "test", "name LIKE
'%test%'"),
+ (ColumnOperatorEnum.ilike, TestModel.name, "test", "name ILIKE
'%test%'"),
+ (ColumnOperatorEnum.is_null, TestModel.name, None, "name IS NULL"),
+ (ColumnOperatorEnum.is_not_null, TestModel.name, None, "name IS NOT
NULL"),
+ ]
+
+ for operator, column, value, expected_sql_fragment in test_cases:
+ # Apply the operator
+ result = operator.apply(column, value)
+
+ # Convert to string to check SQL generation
+ sql_str = str(result.compile(compile_kwargs={"literal_binds": True}))
+
+ # Verify the SQL contains the expected fragment
+ # Note: SQLAlchemy might format this differently, so we check for key
parts
+ if "=" in expected_sql_fragment:
+ assert "=" in sql_str
+ elif "!=" in expected_sql_fragment:
+ assert "!=" in sql_str
+ elif "LIKE" in expected_sql_fragment:
+ assert "LIKE" in sql_str
+ elif "ILIKE" in expected_sql_fragment:
+ assert "ILIKE" in sql_str
+ elif "IN" in expected_sql_fragment:
+ assert "IN" in sql_str
+ elif "NOT IN" in expected_sql_fragment:
+ assert "NOT IN" in sql_str
+ elif ">" in expected_sql_fragment:
+ assert ">" in sql_str
+ elif ">=" in expected_sql_fragment:
+ assert ">=" in sql_str
+ elif "<" in expected_sql_fragment:
+ assert "<" in sql_str
+ elif "<=" in expected_sql_fragment:
+ assert "<=" in sql_str
+ elif "IS NULL" in expected_sql_fragment:
+ assert "IS NULL" in sql_str
+ elif "IS NOT NULL" in expected_sql_fragment:
+ assert "IS NOT NULL" in sql_str
Review Comment:
You're right - these assertions are way too weak. They just check if
certain characters exist anywhere in the SQL string, which could lead
to false positives. Let me rewrite this test to properly validate the
SQL generation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
