refactor: ♻️ change GQL custom test class to use transation #361
Conversation
|
|
@delcroip Just only to double check, could you verify two security issues reported by SONAR? |
| def _fixture_teardown(self): | ||
| with transaction.atomic(), connection.cursor() as cursor: | ||
| for table_name in TABLE_TO_FLUSH: | ||
| cursor.execute(f'TRUNCATE TABLE "{table_name}"') | ||
|
|
||
| for app in apps.get_app_configs(): | ||
| models_dict = app.models | ||
| # Print model names and their classes | ||
| for model_name, model_class in models_dict.items(): | ||
| if ( | ||
| model_name.lower() not in MODEL_NOT_TO_FLUSH | ||
| and model_class._meta.db_table | ||
| and not 'View' in model_class._meta.db_table | ||
| and not model_class._meta.swapped | ||
| ): | ||
| savepoint = transaction.savepoint() | ||
| try: | ||
| cursor.execute(f'TRUNCATE TABLE "{model_class._meta.db_table}"') | ||
| except Exception as e: | ||
| # Roll back to the savepoint (only this operation will be undone) | ||
| transaction.savepoint_rollback(savepoint) |
There was a problem hiding this comment.
This feels like a hack. Can we run tests without --keepdb but instead setup db in a custom class SeededTestRunner(DiscoverRunner) and inside setup_databases setup MODEL_NOT_TO_FLUSH using fixtures?
There was a problem hiding this comment.
yes but we will need a proper way to init the db, we are working on that with solution building
| return GraphQLTestCase.query.__call__(self, query, **kwargs) | ||
|
|
||
| class BaseTestContext: | ||
| def __init__(self, user): | ||
| self.user = user | ||
| # client = None | ||
| def setUp(self): | ||
| # cls.client=Client(cls.schema) | ||
| GraphQLTestCase.setUp.__call__(self) | ||
| super().setUp() |
There was a problem hiding this comment.
The fact that this implementation is inheriting from one class and reaching out to another class that is meant to be inherited from with __call__ multiple times, creates a code smell.
There was a problem hiding this comment.
that the only way I found in python to call an external with another context, inheritance don't work because GraphQLTestCase inherite for TestCase, which inherite from TransactionTestCase .... yes it smell
No description provided.