bito-code-review[bot] commented on PR #43883:
URL: https://github.com/apache/superset/pull/43883#issuecomment-5562530163
<!-- Bito Reply -->
The flagged issue is correct. The current test mock `mock_db` is a
`MagicMock` that does not automatically track or enforce the state of the
SQLAlchemy session, meaning `mock_db.session.rollback.assert_called_once()`
would pass even if the actual `db.session` in the application code were not
properly managed or if the session state was not correctly simulated. To
resolve this, you should ensure the test verifies that the session rollback is
actually triggered on the real session object or use a more robust mock that
simulates the session's state transitions.
Would you like me to implement a fix for this test and check the rest of the
comments on this PR?
**tests/unit_tests/models/test_virtual_dataset_format.py**
```
@patch("superset.models.helpers.db")
# ...
def test_get_from_clause_rolls_back_session_on_rls_failure(
self,
mock_apply_rls: MagicMock,
mock_get_predicates: MagicMock,
mock_db: MagicMock,
virtual_datasource: MagicMock,
app: Flask,
) -> None:
_set_virtual_sql(virtual_datasource, "SELECT pen_id FROM
public.pens")
with pytest.raises(QueryObjectValidationError):
virtual_datasource.get_from_clause(template_processor=None)
mock_db.session.rollback.assert_called_once()
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
