From aa1c1b0bcb5327894c7b00d0454eebae06eaa005 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Sun, 3 Apr 2022 21:53:23 +0000 Subject: [PATCH] [FIX] website, *: correct wrong query count after httpocalypse *: test_website, website_blog (test perfs) An error regarding tests was not catch during httpocalypse [1] review. The test query counts were lowered in website by 2 everywhere, but it shouldn't have as there were no SQL perf improvement from this refactoring regarding website. The query count actually decreased because the test-only SQL Savepoints got divided by 2, probably thanks to a low level test improvement in the PR. But outside of tests, there were no SQL Query gained. This should have been reflected in the `EXTRA_REQUEST` variable instead. Before: Savepoint > Rollback > Savepoint > [Actual SQL Queries] > Rollback After: Savepoint > [Actual SQL Queries] > Rollback It is important to fix it because: 1. (Minor) There is a de-sync between the query count in the test and the actual query count (in the logs outside of tests), making it hard to figure. 2. (Major) Some controller got then wrongly lowered to 0 query counts instead of 2. An incoming PR in master would then make those tests expects negative query counts, which doesn't make any sense. [1]: https://github.com/odoo/odoo/pull/78857 Note that the exact commit can't be found by bisect as those don't work on their own, DB can't be started due to circular import. Part-of: odoo/odoo#87856 --- addons/test_website/tests/test_performance.py | 4 +- addons/website/tests/test_performance.py | 59 +++++++++---------- addons/website_blog/tests/test_performance.py | 20 +++---- 3 files changed, 40 insertions(+), 43 deletions(-) diff --git a/addons/test_website/tests/test_performance.py b/addons/test_website/tests/test_performance.py index 49856546221..88170d22fea 100644 --- a/addons/test_website/tests/test_performance.py +++ b/addons/test_website/tests/test_performance.py @@ -7,5 +7,5 @@ from odoo.addons.website.tests.test_performance import UtilPerf class TestPerformance(UtilPerf): def test_10_perf_sql_website_controller_minimalist(self): url = '/empty_controller_test' - self.assertEqual(self._get_url_hot_query(url), 0) - self.assertEqual(self._get_url_hot_query(url, cache=False), 0) + self.assertEqual(self._get_url_hot_query(url), 2) + self.assertEqual(self._get_url_hot_query(url, cache=False), 2) diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index f62e0de5d67..2f5a019da36 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -3,16 +3,13 @@ from odoo.tests.common import HttpCase -EXTRA_REQUEST = 4 - 1 +EXTRA_REQUEST = 2 - 1 """ During tests, the query on 'base_registry_signaling, base_cache_signaling' -won't be executed on hot state, but new queries related to the test cursor -will be added:: +won't be executed on hot state, but new queries related to the test cursor will +be added: cr = Cursor() # SAVEPOINT - cr.commit() # RELEASE - cr.close() - cr = Cursor() - cr.execute(...) # SAVEPOINT + cr.execute(...) cr.commit() # RELEASE cr.close() """ @@ -39,23 +36,23 @@ class TestStandardPerformance(UtilPerf): # not published user, get the not found image placeholder self.assertEqual(self.env['res.users'].sudo().browse(2).website_published, False) url = '/web/image/res.users/2/image_256' - self.assertEqual(self._get_url_hot_query(url), 4) - self.assertEqual(self._get_url_hot_query(url, cache=False), 4) + self.assertEqual(self._get_url_hot_query(url), 6) + self.assertEqual(self._get_url_hot_query(url, cache=False), 6) def test_11_perf_sql_img_controller(self): self.authenticate('demo', 'demo') self.env['res.users'].sudo().browse(2).website_published = True url = '/web/image/res.users/2/image_256' - self.assertEqual(self._get_url_hot_query(url), 3) - self.assertEqual(self._get_url_hot_query(url, cache=False), 3) + self.assertEqual(self._get_url_hot_query(url), 5) + self.assertEqual(self._get_url_hot_query(url, cache=False), 5) def test_20_perf_sql_img_controller_bis(self): url = '/web/image/website/1/favicon' - self.assertEqual(self._get_url_hot_query(url), 2) - self.assertEqual(self._get_url_hot_query(url, cache=False), 2) + self.assertEqual(self._get_url_hot_query(url), 4) + self.assertEqual(self._get_url_hot_query(url, cache=False), 4) self.authenticate('portal', 'portal') - self.assertEqual(self._get_url_hot_query(url), 2) - self.assertEqual(self._get_url_hot_query(url, cache=False), 2) + self.assertEqual(self._get_url_hot_query(url), 4) + self.assertEqual(self._get_url_hot_query(url, cache=False), 4) class TestWebsitePerformance(UtilPerf): @@ -92,31 +89,31 @@ class TestWebsitePerformance(UtilPerf): def test_10_perf_sql_queries_page(self): # standard untracked website.page - self.assertEqual(self._get_url_hot_query(self.page.url), 5) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 9) + self.assertEqual(self._get_url_hot_query(self.page.url), 7) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11) self.menu.unlink() - self.assertEqual(self._get_url_hot_query(self.page.url), 5) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 9) + self.assertEqual(self._get_url_hot_query(self.page.url), 7) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11) def test_15_perf_sql_queries_page(self): # standard tracked website.page self.page.track = True - self.assertEqual(self._get_url_hot_query(self.page.url), 13) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 17) + self.assertEqual(self._get_url_hot_query(self.page.url), 15) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 19) self.menu.unlink() - self.assertEqual(self._get_url_hot_query(self.page.url), 13) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 17) + self.assertEqual(self._get_url_hot_query(self.page.url), 15) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 19) def test_20_perf_sql_queries_homepage(self): # homepage "/" has its own controller - self.assertEqual(self._get_url_hot_query('/'), 12) - self.assertEqual(self._get_url_hot_query('/', cache=False), 16) + self.assertEqual(self._get_url_hot_query('/'), 14) + self.assertEqual(self._get_url_hot_query('/', cache=False), 18) def test_30_perf_sql_queries_page_no_layout(self): # website.page with no call to layout templates self.page.arch = '
I am a blank page
' - self.assertEqual(self._get_url_hot_query(self.page.url), 5) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 5) + self.assertEqual(self._get_url_hot_query(self.page.url), 7) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 7) def test_40_perf_sql_queries_page_multi_level_menu(self): # menu structure should not impact SQL requests @@ -134,12 +131,12 @@ class TestWebsitePerformance(UtilPerf): menu_bb.parent_id = menu_b menu_aa.parent_id = menu_a - self.assertEqual(self._get_url_hot_query(self.page.url), 5) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 9) + self.assertEqual(self._get_url_hot_query(self.page.url), 7) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11) def test_50_perf_sql_web_assets(self): # assets route /web/assets/.. self.url_open('/') # create assets attachments assets_url = self.env['ir.attachment'].search([('url', '=like', '/web/assets/%/web.assets_common%.js')], limit=1).url - self.assertEqual(self._get_url_hot_query(assets_url), 0) - self.assertEqual(self._get_url_hot_query(assets_url, cache=False), 0) + self.assertEqual(self._get_url_hot_query(assets_url), 2) + self.assertEqual(self._get_url_hot_query(assets_url, cache=False), 2) diff --git a/addons/website_blog/tests/test_performance.py b/addons/website_blog/tests/test_performance.py index eb7acccabcf..7795024691a 100644 --- a/addons/website_blog/tests/test_performance.py +++ b/addons/website_blog/tests/test_performance.py @@ -13,8 +13,8 @@ class TestBlogPerformance(UtilPerf): self.env['website'].search([]).channel_id = False def test_10_perf_sql_blog_standard_data(self): - self.assertEqual(self._get_url_hot_query('/blog'), 26) - self.assertEqual(self._get_url_hot_query('/blog', cache=False), 25) + self.assertEqual(self._get_url_hot_query('/blog'), 28) + self.assertEqual(self._get_url_hot_query('/blog', cache=False), 27) def test_20_perf_sql_blog_bigger_data_scaling(self): BlogPost = self.env['blog.post'] @@ -26,10 +26,10 @@ class TestBlogPerformance(UtilPerf): for blog_post in blog_posts: blog_post.tag_ids += blog_tags blog_tags = blog_tags[:-1] - self.assertEqual(self._get_url_hot_query('/blog'), 26) - self.assertEqual(self._get_url_hot_query('/blog', cache=False), 25) - self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 30) - self.assertEqual(self._get_url_hot_query(blog_post[0].website_url, cache=False), 29) + self.assertEqual(self._get_url_hot_query('/blog'), 28) + self.assertEqual(self._get_url_hot_query('/blog', cache=False), 27) + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 32) + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url, cache=False), 31) def test_30_perf_sql_blog_bigger_data_scaling(self): BlogPost = self.env['blog.post'] @@ -41,7 +41,7 @@ class TestBlogPerformance(UtilPerf): for blog_post in blog_posts: blog_post.write({'tag_ids': [[6, 0, random.choices(blog_tags.ids, k=random.randint(0, len(blog_tags)))]]}) - self.assertLessEqual(self._get_url_hot_query('/blog'), 27) - self.assertLessEqual(self._get_url_hot_query('/blog', cache=False), 26) - self.assertLessEqual(self._get_url_hot_query(blog_post[0].website_url), 32) - self.assertLessEqual(self._get_url_hot_query(blog_post[0].website_url, cache=False), 31) + self.assertLessEqual(self._get_url_hot_query('/blog'), 29) + self.assertLessEqual(self._get_url_hot_query('/blog', cache=False), 28) + self.assertLessEqual(self._get_url_hot_query(blog_post[0].website_url), 34) + self.assertLessEqual(self._get_url_hot_query(blog_post[0].website_url, cache=False), 33)