Re: [PR] chore(export): Added ability to export chart YAML files with Unicode characters, fix #20331 [superset]

["xyb (via GitHub)" <gi...@apache.org>]

xyb commented on code in PR #28008:
URL: https://github.com/apache/superset/pull/28008#discussion_r1739963068


##########
tests/integration_tests/datasets/commands_tests.py:
##########
@@ -587,6 +618,7 @@ def test_create_dataset_command(self):
             self.assertEqual([owner.username for owner in table.owners], ["admin"])
 
         db.session.delete(table)
+        db.session.commit()

Review Comment:
   This test case will raise `sqlalchemy.exc.OperationalError` exception if the commit operation is not included:
   ```python
       def test_create_dataset_command(self):
           ...
           with examples_db.get_sqla_engine() as engine:
               engine.execute("DROP TABLE IF EXISTS test_create_dataset_command")
               engine.execute(
                   "CREATE TABLE test_create_dataset_command AS SELECT 2 as col"
               )
           ...
           db.session.delete(table)
           with examples_db.get_sqla_engine() as engine:
   >           engine.execute("DROP TABLE test_create_dataset_command")
   
       def do_execute(self, cursor, statement, parameters, context=None):
   >       cursor.execute(statement, parameters)
   E       sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) database is locked
   E       [SQL: DROP TABLE test_create_dataset_command]
   E       (Background on this error at: https://sqlalche.me/e/14/e3q8)
   ```
   
   After reviewing the code, I realized that the test case requires committing the new table before dropping it. I'm not sure why this wasn't caught earlier, but I've double-checked the new test case I added, which includes all necessary commits. To ensure the unit test passes, I've made this change. If I've missed anything, please let me know and I'll be happy to make adjustments.



-- 
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-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org