From 236c84ab8bbba439057149c9a20eb289f7b85b52 Mon Sep 17 00:00:00 2001 From: abhishek Date: Mon, 18 Oct 2021 16:35:13 +0530 Subject: [PATCH 1/5] feat: semgrep rule for db.sql --- .github/helper/semgrep_rules/frappe_correctness.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index d9603e89aa..04341b3430 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -131,3 +131,10 @@ rules: key `$X` is uselessly assigned twice. This could be a potential bug. languages: [python] severity: ERROR + +- id: frappe-using-db.sql + pattern-regex: \.sql.*\( + message: | + The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database) + languages: [python] + severity: ERROR \ No newline at end of file From 059e5441c27baf0a81b2356359eabd1e3eb354ab Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 18 Oct 2021 21:33:06 +0530 Subject: [PATCH 2/5] ci: use semgrep pattern instead of regex --- .github/helper/semgrep_rules/frappe_correctness.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 04341b3430..662e6e8022 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -132,8 +132,8 @@ rules: languages: [python] severity: ERROR -- id: frappe-using-db.sql - pattern-regex: \.sql.*\( +- id: frappe-using-db-sql + pattern: frappe.db.sql(...) message: | The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database) languages: [python] From ac1bb636cf86490702dc6330d4c7e01937bb5d13 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 18 Oct 2021 22:11:38 +0530 Subject: [PATCH 3/5] ci: flag new instances of sql_ddl and sql_list --- .github/helper/semgrep_rules/frappe_correctness.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 662e6e8022..1297d7fa67 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -133,8 +133,11 @@ rules: severity: ERROR - id: frappe-using-db-sql - pattern: frappe.db.sql(...) + pattern-either: + - pattern: frappe.db.sql(...) + - pattern: frappe.db.sql_ddl(...) + - pattern: frappe.db.sql_list(...) message: | The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database) languages: [python] - severity: ERROR \ No newline at end of file + severity: ERROR From a575cff8a7f55fc5550bd6a519e3e6288b9200ee Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 19 Oct 2021 12:35:55 +0530 Subject: [PATCH 4/5] fix: Retry flaky test_printview_page if errored --- frappe/tests/test_website.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index 818dc8bce6..688679a80f 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -4,6 +4,7 @@ import frappe from frappe.utils import set_request from frappe.website.serve import get_response, get_response_content from frappe.website.utils import (build_response, clear_website_cache, get_home_page) +from tenacity import retry, stop_after_attempt, retry_if_exception_type class TestWebsite(unittest.TestCase): @@ -196,6 +197,11 @@ class TestWebsite(unittest.TestCase): delattr(frappe.hooks, 'page_renderer') frappe.cache().delete_key('app_hooks') + # TODO: Get rid of this retry logic + # Added since test is flaky and we can't figure out why at this point + @retry( + stop=stop_after_attempt(5), retry=retry_if_exception_type(AssertionError), + ) def test_printview_page(self): content = get_response_content('/Language/en') self.assertIn('
', content) From 4a5bb2873dc900f7c4394f6cb5ecf6d5170cfb18 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 19 Oct 2021 13:25:37 +0530 Subject: [PATCH 5/5] ci: Ignore sql=>qb rule for test files --- .github/helper/semgrep_rules/frappe_correctness.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml index 1297d7fa67..33a22fba6a 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.yml +++ b/.github/helper/semgrep_rules/frappe_correctness.yml @@ -137,6 +137,9 @@ rules: - pattern: frappe.db.sql(...) - pattern: frappe.db.sql_ddl(...) - pattern: frappe.db.sql_list(...) + paths: + exclude: + - "test_*.py" message: | The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database) languages: [python]