Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Django 4.2b1 is released and some tests are failing #156

Open
gurkanindibay opened this issue Feb 24, 2023 · 1 comment
Open

Django 4.2b1 is released and some tests are failing #156

gurkanindibay opened this issue Feb 24, 2023 · 1 comment

Comments

@gurkanindibay
Copy link
Contributor

gurkanindibay commented Feb 24, 2023

https://github.com/citusdata/django-multitenant/actions/runs/4262532233

@apagano-vue
Copy link

apagano-vue commented Jun 13, 2023

@gurkanindibay : I've looked into this to make sure this lib would work as expected with our version of Django (4.2.2) and it seems the failures are caused by two different issues: 

  1. In version 4.2 the django backend db logger (used in the built-in assertNumQueries helper) now logs BEGIN, COMMIT, and ROLLBACK operations (see https://docs.djangoproject.com/en/4.2/releases/4.2/#logging). This adds two additional captured queries (BEGIN+COMMIT) for all the test involving a deletion (test_delete_tenant_set, test_delete_cascade_distributed, test_delete_cascade_distributed, test_delete_cascade_reference_to_distributed, test_delete_cascade_distributed_to_reference and test_delete_tenant_set). For those tests the fix is simple: add 2 to the expected count of queries. The following also needs to be added to test_delete_tenant_set to avoid failing the assertion because of those two additional queries: 
            for query in captured_queries.captured_queries:
                if "BEGIN" == query["sql"] or "COMMIT" == query["sql"]:
                    continue
                if "tests_revenue" in query["sql"]:
                    self.assertTrue(f'"acc_id" = {account.id}' in query["sql"])
                else:
                    self.assertTrue(f'"account_id" = {account.id}' in query["sql"])
  1. Even with the fix for point 1 test_delete_cascade_distributed_to_reference still fails. A quick diff between the output of running the test with Django 4.1 and django 4.2 shows this: 

Django 4.1: 

...
E   4. SELECT "tests_project"."id" FROM "tests_project" WHERE ("tests_project"."employee_id" IN (4) AND "tests_project"."account_id" = 1)
...
E   6. SELECT "tests_project"."id" FROM "tests_project" WHERE ("tests_project"."account_id" IN (1) AND "tests_project"."account_id" = 1)
...

Django 4.2: 

...
E   4. SELECT "tests_project"."id" FROM "tests_project" WHERE ("tests_project"."account_id" IN (1) AND "tests_project"."account_id" = 1)
...

So the first SELECT statement is not happening anymore.

I would be happy to make a PR to resolve 1 but I'm afraid I am not familiar enough with the library to know what is causing 2 (is it a bug/breaking change that should be fixed or just a minor change as is point 1?), how critical it is and what should be done about it.

Let me know what you think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants