From d8e91cae329d69f419d9ac4ea3bed57155a98d3a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 23 Apr 2021 01:20:47 +0530 Subject: [PATCH 01/42] fix: Strip comments before sanitizing column_name --- frappe/utils/data.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 3ffa8dc874..dbf0d0665a 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1278,7 +1278,9 @@ def make_filter_dict(filters): def sanitize_column(column_name): from frappe import _ + import sqlparse regex = re.compile("^.*[,'();].*") + column_name = sqlparse.format(column_name, strip_comments=True, keyword_case="lower") blacklisted_keywords = ['select', 'create', 'insert', 'delete', 'drop', 'update', 'case', 'and', 'or'] def _raise_exception(): From 249fa6d21b343342ec62ac19597e847cc656c10b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 2 May 2021 14:28:16 +0530 Subject: [PATCH 02/42] feat: Add run-parallel-tests command - Tests will be distributed across different build machines (dependent on test orchestrator) - PrettyPrint for test results --- frappe/commands/utils.py | 11 ++- frappe/test_runner.py | 192 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 1 deletion(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index c4b6cf4655..4374b77a66 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -552,6 +552,14 @@ def run_tests(context, app=None, module=None, doctype=None, test=(), profile=Fal if os.environ.get('CI'): sys.exit(ret) +@click.command('run-parallel-tests') +@click.option('--app', help="For App", default='frappe') +@click.option('--build-id', help="For App") +@pass_context +def run_parallel_tests(context, app, build_id): + from frappe.test_runner import ParallelTestRunner + site = get_site(context) + ParallelTestRunner(app, site=site, build_id=build_id) @click.command('run-ui-tests') @click.argument('app') @@ -801,5 +809,6 @@ commands = [ watch, bulk_rename, add_to_email_queue, - rebuild_global_search + rebuild_global_search, + run_parallel_tests ] diff --git a/frappe/test_runner.py b/frappe/test_runner.py index fd8e38e587..1c48303f04 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -15,6 +15,8 @@ import cProfile, pstats from six import StringIO from six.moves import reload_module from frappe.model.naming import revert_series_if_last +import click +import requests unittest_runner = unittest.TextTestRunner SLOW_TEST_THRESHOLD = 2 @@ -422,3 +424,193 @@ def get_test_record_log(): frappe.flags.test_record_log = [] return frappe.flags.test_record_log + +class Writeln(object): + def __init__(self,stream): + self.stream = stream + + def __getattr__(self, attr): + if attr in ('stream', '__getstate__'): + raise AttributeError(attr) + return getattr(self.stream,attr) + + def writeln(self, arg=None): + if arg: + self.write(arg) + self.write('\n') + +class PrettyPrintResult(unittest.TextTestResult): + def startTest(self, test): + super(unittest.TextTestResult, self).startTest(test) + click.echo('\n') + + def addSuccess(self, test): + super(unittest.TextTestResult, self).addSuccess(test) + click.echo("%s %s" % (click.style(' PASS ', bg='green', fg='black'), self.getDescription(test))) + + def addError(self, test, err): + super(unittest.TextTestResult, self).addError(test, err) + click.echo("%s %s" % (click.style(' ERROR ', bg='red', fg='white'), self.getDescription(test))) + + def addFailure(self, test, err): + super(unittest.TextTestResult, self).addFailure(test, err) + click.echo("%s %s" % (click.style(' FAIL ', bg='red', fg='white'), self.getDescription(test))) + click.echo('\n') + + def printErrors(self): + if self.dots or self.showAll: + self.stream.writeln() + self.printErrorList(' ERROR ', self.errors, 'red') + self.printErrorList(' FAIL ', self.failures, 'red') + + def printErrorList(self, flavour, errors, color): + for test, err in errors: + click.echo(self.separator1) + click.echo("%s %s" % (click.style(flavour, bg=color), self.getDescription(test))) + click.echo(self.separator2) + click.echo("%s" % err) + + def __repr__(self): + return f"run={self.testsRun} errors={len(self.errors)} failures={len(self.failures)}" + +def get_all_tests(): + test_file_list = [] + for path, folders, files in os.walk(frappe.get_pymodule_path('frappe')): + for dontwalk in ('locals', '.git', 'public', '__pycache__'): + if dontwalk in folders: + folders.remove(dontwalk) + + # for predictability + folders.sort() + files.sort() + + # print path + for filename in files: + if filename.startswith("test_") and filename.endswith(".py")\ + and filename != 'test_runner.py': + test_file_list.append(os.path.join(path, filename)) + return test_file_list + +class ParallelTestRunner(): + def __init__(self, app, site, build_id, instance_id=None): + self.app = app + self.site = site + self.orchestrator_url = 'http://localhost:3000' + self.build_id = build_id or '12321' + self.setup_test_site() + self.instance_id = instance_id or frappe.generate_hash(length=10) + frappe.flags.in_test = True + self.start_test() + + def setup_test_site(self): + frappe.init(site=self.site) + if not frappe.db: + frappe.connect() + + frappe.clear_cache() + frappe.utils.scheduler.disable_scheduler() + set_test_email_config() + + def run_before_test_hook(self): + for fn in frappe.get_hooks("before_tests", app_name=self.app): + frappe.get_attr(fn)() + + def start_test(self): + self.register_instance() + self.test_result = PrettyPrintResult(stream=Writeln(sys.stderr), descriptions=True, verbosity=2) + self.test_status = 'ongoing' + + while self.test_status == 'ongoing': + self.run_tests_for_file(self.get_next_test()) + + self.print_result() + + def register_instance(self): + test_spec_list = get_all_tests() + self.call_orchestrator('init-test', data={ + 'test_spec_list': test_spec_list + }) + + def get_next_test(self): + response_data = self.call_orchestrator('get-next-test') + self.test_status = response_data.get('status') + return response_data.get('next_test') + + def run_tests_for_file(self, file_path): + if not file_path: + return + + app = self.app + filename = file_path.split('/')[-1] + path = file_path.rsplit('/', 1)[0] + + if os.path.sep.join(["doctype", "doctype", "boilerplate"]) in path: + # in /doctype/doctype/boilerplate/ + return + + app_path = frappe.get_pymodule_path(app) + relative_path = os.path.relpath(path, app_path) + if relative_path == '.': + module_name = app + else: + module_name = '{app}.{relative_path}.{module_name}'.format(app=app, + relative_path=relative_path.replace('/', '.'), module_name=filename[:-3]) + + module = importlib.import_module(module_name) + if hasattr(module, "test_dependencies"): + for doctype in module.test_dependencies: + make_test_records(doctype) + + test_suite = unittest.TestSuite() + module_test_cases = unittest.TestLoader().loadTestsFromModule(module) + test_suite.addTest(module_test_cases) + test_suite(self.test_result) + + def print_result(self): + self.test_result.printErrors() + click.echo(self.test_result) + + def call_orchestrator(self, endpoint, data={}): + # add repo token header + # build id in header + res = requests.get(f'{self.orchestrator_url}/{endpoint}', data=data, headers={ + 'CI-BUILD-ID': self.build_id, + 'CI-INSTANCE-ID': self.instance_id, + 'REPO-TOKEN': '2948288382838DE' + }) + res.raise_for_status() + return res.json() if 'application/json' in res.headers.get('content-type') else {} + + # def setup_coverage(self): + # if self.with_coverage: + # from coverage import Coverage + # from frappe.utils import get_bench_path + + # # Generate coverage report only for app that is being tested + # source_path = os.path.join(get_bench_path(), 'apps', self.app) + # omit=[ + # '*.html', + # '*.js', + # '*.xml', + # '*.css', + # '*.less', + # '*.scss', + # '*.vue', + # '*/doctype/*/*_dashboard.py', + # '*/patches/*' + # ] + + # if self.app == 'frappe': + # omit.append('*/commands/*') + + # self.coverage = Coverage(source=[source_path], omit=omit, data_file='coverage_report') + # self.cov.start() + + # def submit_coverage(self): + # if self.with_coverage: + # self.cov.stop() + + # if self.is_master: + # pass + +# [] coverage From 7ac3b53c1c8fb902d430301c5b1ee47525d19560 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 2 May 2021 21:01:47 +0530 Subject: [PATCH 03/42] feat: Add support for build combined coverage file - from parallel tests - Update workflow files --- .github/workflows/ci-tests.yml | 171 -------------------- .github/workflows/server-mariadb-tests.yml | 123 ++++++++++++++ .github/workflows/server-postgres-tests.yml | 97 +++++++++++ frappe/commands/utils.py | 7 +- frappe/test_runner.py | 122 +++++++++----- 5 files changed, 308 insertions(+), 212 deletions(-) delete mode 100644 .github/workflows/ci-tests.yml create mode 100644 .github/workflows/server-mariadb-tests.yml create mode 100644 .github/workflows/server-postgres-tests.yml diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml deleted file mode 100644 index d2fbef528b..0000000000 --- a/.github/workflows/ci-tests.yml +++ /dev/null @@ -1,171 +0,0 @@ -name: CI - -on: - pull_request: - types: [opened, synchronize, reopened, labeled, unlabeled] - workflow_dispatch: - push: - -jobs: - test: - runs-on: ubuntu-18.04 - - strategy: - fail-fast: false - matrix: - include: - - DB: "mariadb" - TYPE: "server" - JOB_NAME: "Python MariaDB" - RUN_COMMAND: bench --site test_site run-tests --coverage - - - DB: "postgres" - TYPE: "server" - JOB_NAME: "Python PostgreSQL" - RUN_COMMAND: bench --site test_site run-tests --coverage - - - DB: "mariadb" - TYPE: "ui" - JOB_NAME: "UI MariaDB" - RUN_COMMAND: bench --site test_site run-ui-tests frappe --headless - - name: ${{ matrix.JOB_NAME }} - - services: - mysql: - image: mariadb:10.3 - env: - MYSQL_ALLOW_EMPTY_PASSWORD: YES - ports: - - 3306:3306 - options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3 - - postgres: - image: postgres:12.4 - env: - POSTGRES_PASSWORD: travis - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - ports: - - 5432:5432 - - steps: - - name: Clone - uses: actions/checkout@v2 - - - name: Setup Python - uses: actions/setup-python@v2 - with: - python-version: 3.7 - - - uses: actions/setup-node@v2 - with: - node-version: '12' - check-latest: true - - - name: Add to Hosts - run: | - echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts - echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts - - - name: Cache pip - uses: actions/cache@v2 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} - restore-keys: | - ${{ runner.os }}-pip- - ${{ runner.os }}- - - - name: Cache node modules - uses: actions/cache@v2 - env: - cache-name: cache-node-modules - with: - path: ~/.npm - key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} - restore-keys: | - ${{ runner.os }}-build-${{ env.cache-name }}- - ${{ runner.os }}-build- - ${{ runner.os }}- - - - name: Get yarn cache directory path - id: yarn-cache-dir-path - run: echo "::set-output name=dir::$(yarn cache dir)" - - - uses: actions/cache@v2 - id: yarn-cache - with: - path: ${{ steps.yarn-cache-dir-path.outputs.dir }} - key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} - restore-keys: | - ${{ runner.os }}-yarn- - - - name: Cache cypress binary - if: matrix.TYPE == 'ui' - uses: actions/cache@v2 - with: - path: ~/.cache - key: ${{ runner.os }}-cypress- - restore-keys: | - ${{ runner.os }}-cypress- - ${{ runner.os }}- - - - name: Install Dependencies - run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh - env: - BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} - AFTER: ${{ env.GITHUB_EVENT_PATH.after }} - TYPE: ${{ matrix.TYPE }} - - - name: Install - run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh - env: - DB: ${{ matrix.DB }} - TYPE: ${{ matrix.TYPE }} - - - name: Run Set-Up - if: matrix.TYPE == 'ui' - run: cd ~/frappe-bench/ && bench --site test_site execute frappe.utils.install.complete_setup_wizard - env: - DB: ${{ matrix.DB }} - TYPE: ${{ matrix.TYPE }} - - - name: Setup tmate session - if: contains(github.event.pull_request.labels.*.name, 'debug-gha') - uses: mxschmitt/action-tmate@v3 - - - name: Run Tests - run: cd ~/frappe-bench/ && ${{ matrix.RUN_COMMAND }} - env: - DB: ${{ matrix.DB }} - TYPE: ${{ matrix.TYPE }} - - - name: Coverage - Pull Request - if: matrix.TYPE == 'server' && github.event_name == 'pull_request' - run: | - cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE} - cd ${GITHUB_WORKSPACE} - pip install coveralls==2.2.0 - pip install coverage==4.5.4 - coveralls --service=github - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} - COVERALLS_SERVICE_NAME: github - - - name: Coverage - Push - if: matrix.TYPE == 'server' && github.event_name == 'push' - run: | - cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE} - cd ${GITHUB_WORKSPACE} - pip install coveralls==2.2.0 - pip install coverage==4.5.4 - coveralls --service=github-actions - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} - COVERALLS_SERVICE_NAME: github-actions diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml new file mode 100644 index 0000000000..a032025bae --- /dev/null +++ b/.github/workflows/server-mariadb-tests.yml @@ -0,0 +1,123 @@ +name: Server Mariadb tests + +on: + pull_request: + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-18.04 + + strategy: + fail-fast: false + matrix: + containers: [1, 2] + + name: Server Mariadb + + services: + mysql: + image: mariadb:10.3 + env: + MYSQL_ALLOW_EMPTY_PASSWORD: YES + ports: + - 3306:3306 + options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3 + + steps: + - name: Clone + uses: actions/checkout@v2 + + - name: Setup Python + uses: actions/setup-python@v2 + with: + python-version: 3.7 + + - uses: actions/setup-node@v2 + with: + node-version: '14' + check-latest: true + + - name: Add to Hosts + run: | + echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts + echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts + + - name: Cache pip + uses: actions/cache@v2 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + ${{ runner.os }}- + + - name: Cache node modules + uses: actions/cache@v2 + env: + cache-name: cache-node-modules + with: + path: ~/.npm + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-build-${{ env.cache-name }}- + ${{ runner.os }}-build- + ${{ runner.os }}- + + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "::set-output name=dir::$(yarn cache dir)" + + - uses: actions/cache@v2 + id: yarn-cache + with: + path: ${{ steps.yarn-cache-dir-path.outputs.dir }} + key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} + restore-keys: | + ${{ runner.os }}-yarn- + + - name: Install Dependencies + run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh + env: + BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} + AFTER: ${{ env.GITHUB_EVENT_PATH.after }} + TYPE: server + + - name: Install + run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh + env: + DB: mariadb + TYPE: server + + - name: Setup tmate session + if: contains(github.event.pull_request.labels.*.name, 'debug-gha') + uses: mxschmitt/action-tmate@v3 + + - name: Run Tests + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --with-coverage --ci-build-id $GITHUB_RUN_NUMBER + + # - name: Coverage - Pull Request + # if: github.event_name == 'pull_request' + # run: | + # cp ~/frappe-bench/sites/combined_coverage ${GITHUB_WORKSPACE} + # cd ${GITHUB_WORKSPACE} + # if [ -f "./combined_coverage" ]; pip install coveralls==2.2.0 && pip install coverage==4.5.4; fi + # if [ -f "./combined_coverage" ]; coveralls --service=github --source ./combined_coverage; fi + # coveralls --service=github + # env: + # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} + # COVERALLS_SERVICE_NAME: github + + # - name: Coverage - Push + # if: github.event_name == 'push' + # run: | + # cp ~/frappe-bench/sites/combined_coverage ${GITHUB_WORKSPACE}/.coverage + # cd ${GITHUB_WORKSPACE} + # pip install coveralls==2.2.0 + # pip install coverage==4.5.4 + # coveralls --service=github-actions + # env: + # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} + # COVERALLS_SERVICE_NAME: github-actions diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml new file mode 100644 index 0000000000..f47268af69 --- /dev/null +++ b/.github/workflows/server-postgres-tests.yml @@ -0,0 +1,97 @@ +name: Server PostgreSQL tests + +on: + pull_request: + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-18.04 + + strategy: + fail-fast: false + matrix: + containers: [1, 2] + + name: Server PostgreSQL + + services: + postgres: + image: postgres:12.4 + env: + POSTGRES_PASSWORD: travis + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + + steps: + - name: Clone + uses: actions/checkout@v2 + + - name: Setup Python + uses: actions/setup-python@v2 + with: + python-version: 3.7 + + - uses: actions/setup-node@v2 + with: + node-version: '14' + check-latest: true + + - name: Add to Hosts + run: | + echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts + echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts + + - name: Cache pip + uses: actions/cache@v2 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + ${{ runner.os }}- + + - name: Cache node modules + uses: actions/cache@v2 + env: + cache-name: cache-node-modules + with: + path: ~/.npm + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-build-${{ env.cache-name }}- + ${{ runner.os }}-build- + ${{ runner.os }}- + + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "::set-output name=dir::$(yarn cache dir)" + + - uses: actions/cache@v2 + id: yarn-cache + with: + path: ${{ steps.yarn-cache-dir-path.outputs.dir }} + key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} + restore-keys: | + ${{ runner.os }}-yarn- + + - name: Install Dependencies + run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh + env: + BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} + AFTER: ${{ env.GITHUB_EVENT_PATH.after }} + TYPE: server + + - name: Install + run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh + env: + DB: postgres + TYPE: server + + - name: Run Tests + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --ci-build-id $GITHUB_RUN_NUMBER diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 4374b77a66..771c1d81fb 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -554,12 +554,13 @@ def run_tests(context, app=None, module=None, doctype=None, test=(), profile=Fal @click.command('run-parallel-tests') @click.option('--app', help="For App", default='frappe') -@click.option('--build-id', help="For App") +@click.option('--ci-build-id', help="CI Build ID") +@click.option('--with-coverage', is_flag=True, help="Build coverage file") @pass_context -def run_parallel_tests(context, app, build_id): +def run_parallel_tests(context, app, ci_build_id, with_coverage): from frappe.test_runner import ParallelTestRunner site = get_site(context) - ParallelTestRunner(app, site=site, build_id=build_id) + ParallelTestRunner(app, site=site, ci_build_id=ci_build_id, with_coverage=with_coverage) @click.command('run-ui-tests') @click.argument('app') diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 1c48303f04..5207414fe3 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -492,13 +492,14 @@ def get_all_tests(): return test_file_list class ParallelTestRunner(): - def __init__(self, app, site, build_id, instance_id=None): + def __init__(self, app, site, ci_build_id, ci_instance_id=None, with_coverage=False): self.app = app self.site = site - self.orchestrator_url = 'http://localhost:3000' - self.build_id = build_id or '12321' + self.orchestrator_url = 'https://8b52f89a8c13.ngrok.io' + self.ci_build_id = ci_build_id + self.with_coverage = with_coverage self.setup_test_site() - self.instance_id = instance_id or frappe.generate_hash(length=10) + self.ci_instance_id = ci_instance_id or frappe.generate_hash(length=10) frappe.flags.in_test = True self.start_test() @@ -520,16 +521,20 @@ class ParallelTestRunner(): self.test_result = PrettyPrintResult(stream=Writeln(sys.stderr), descriptions=True, verbosity=2) self.test_status = 'ongoing' + self.setup_coverage() while self.test_status == 'ongoing': self.run_tests_for_file(self.get_next_test()) self.print_result() + self.call_orchestrator('test-completed') + self.submit_coverage() def register_instance(self): test_spec_list = get_all_tests() - self.call_orchestrator('init-test', data={ + response_data = self.call_orchestrator('init-test', data={ 'test_spec_list': test_spec_list }) + self.is_master = response_data.get('is_master') def get_next_test(self): response_data = self.call_orchestrator('get-next-test') @@ -570,47 +575,88 @@ class ParallelTestRunner(): self.test_result.printErrors() click.echo(self.test_result) - def call_orchestrator(self, endpoint, data={}): + def call_orchestrator(self, endpoint, data={}, files={}): # add repo token header # build id in header - res = requests.get(f'{self.orchestrator_url}/{endpoint}', data=data, headers={ - 'CI-BUILD-ID': self.build_id, - 'CI-INSTANCE-ID': self.instance_id, + headers = { + 'CI-BUILD-ID': self.ci_build_id, + 'CI-INSTANCE-ID': self.ci_instance_id, 'REPO-TOKEN': '2948288382838DE' - }) + } + url = f'{self.orchestrator_url}/{endpoint}' + + if files: + res = requests.post(url, headers=headers, files=files) + else: + res = requests.get(url, data=data, headers=headers) + print(self.ci_build_id, self.ci_instance_id, endpoint) res.raise_for_status() - return res.json() if 'application/json' in res.headers.get('content-type') else {} + response_data = {} + if 'application/json' in res.headers.get('content-type'): + response_data = res.json() + elif 'application/zip' in res.headers.get('content-type'): + response_data = res.content - # def setup_coverage(self): - # if self.with_coverage: - # from coverage import Coverage - # from frappe.utils import get_bench_path + return response_data - # # Generate coverage report only for app that is being tested - # source_path = os.path.join(get_bench_path(), 'apps', self.app) - # omit=[ - # '*.html', - # '*.js', - # '*.xml', - # '*.css', - # '*.less', - # '*.scss', - # '*.vue', - # '*/doctype/*/*_dashboard.py', - # '*/patches/*' - # ] + def setup_coverage(self): + if self.with_coverage: + from coverage import Coverage + from frappe.utils import get_bench_path - # if self.app == 'frappe': - # omit.append('*/commands/*') + # Generate coverage report only for app that is being tested + source_path = os.path.join(get_bench_path(), 'apps', self.app) + omit=[ + '*.html', + '*.js', + '*.xml', + '*.css', + '*.less', + '*.scss', + '*.vue', + '*/doctype/*/*_dashboard.py', + '*/patches/*' + ] - # self.coverage = Coverage(source=[source_path], omit=omit, data_file='coverage_report') - # self.cov.start() + if self.app == 'frappe': + omit.append('*/commands/*') - # def submit_coverage(self): - # if self.with_coverage: - # self.cov.stop() + self.coverage = Coverage( + source=[source_path], + omit=omit, + data_file='coverage_data', + data_suffix=self.ci_instance_id + ) + self.coverage.start() - # if self.is_master: - # pass + def submit_coverage(self): + if self.with_coverage: + self.coverage.stop() + self.coverage.save() -# [] coverage + if self.is_master: + self.build_coverage_file() + else: + self.upload_coverage_file() + + + def upload_coverage_file(self): + files = {'upload_file': open(f'coverage_data.{self.ci_instance_id}','rb')} + self.call_orchestrator('upload-coverage-file', files=files) + + + def build_coverage_file(self): + import time + import zipfile + import io + click.echo() + while self.call_orchestrator('test-status')['test_status'] == 'ongoing': + click.echo('Waiting for tests to complete...') + time.sleep(5) + + res = self.call_orchestrator('download-coverage-files') + z = zipfile.ZipFile(io.BytesIO(res)) + z.extractall("./coverage_files") + file_list = os.listdir('./coverage_files') + + self.coverage.combine(data_paths=file_list) From cf0e15e2648d68c3baabce8748c919e0f4ca634a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 2 May 2021 21:03:09 +0530 Subject: [PATCH 04/42] feat: Enable cypress parallel testing --- .github/workflows/ui-tests.yml | 106 +++++++++++++++++++++++++++++++++ frappe/commands/utils.py | 10 +++- 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/ui-tests.yml diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml new file mode 100644 index 0000000000..705b51c0ed --- /dev/null +++ b/.github/workflows/ui-tests.yml @@ -0,0 +1,106 @@ +name: UI tests + +on: + pull_request: + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-18.04 + + strategy: + fail-fast: false + matrix: + containers: [1, 2] + + name: UI Mariadb + + services: + mysql: + image: mariadb:10.3 + env: + MYSQL_ALLOW_EMPTY_PASSWORD: YES + ports: + - 3306:3306 + options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3 + + steps: + - name: Clone + uses: actions/checkout@v2 + + - name: Setup Python + uses: actions/setup-python@v2 + with: + python-version: 3.7 + + - uses: actions/setup-node@v2 + with: + node-version: '12' + check-latest: true + + - name: Add to Hosts + run: | + echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts + echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts + + - name: Cache pip + uses: actions/cache@v2 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + ${{ runner.os }}- + + - name: Cache node modules + uses: actions/cache@v2 + env: + cache-name: cache-node-modules + with: + path: ~/.npm + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-build-${{ env.cache-name }}- + ${{ runner.os }}-build- + ${{ runner.os }}- + + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "::set-output name=dir::$(yarn cache dir)" + + - uses: actions/cache@v2 + id: yarn-cache + with: + path: ${{ steps.yarn-cache-dir-path.outputs.dir }} + key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} + restore-keys: | + ${{ runner.os }}-yarn- + + - name: Cache cypress binary + uses: actions/cache@v2 + with: + path: ~/.cache + key: ${{ runner.os }}-cypress- + restore-keys: | + ${{ runner.os }}-cypress- + ${{ runner.os }}- + + - name: Install Dependencies + run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh + env: + BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} + AFTER: ${{ env.GITHUB_EVENT_PATH.after }} + TYPE: 'ui' + + - name: Install + run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh + env: + DB: 'mariadb' + TYPE: 'ui' + + - name: Run Set-Up + if: matrix.TYPE == 'ui' + run: cd ~/frappe-bench/ && bench --site test_site execute frappe.utils.install.complete_setup_wizard + + - name: Run Tests + run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --headless --parallel --ci-build-id $GITHUB_RUN_NUMBER diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 771c1d81fb..df4250cd10 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -565,8 +565,10 @@ def run_parallel_tests(context, app, ci_build_id, with_coverage): @click.command('run-ui-tests') @click.argument('app') @click.option('--headless', is_flag=True, help="Run UI Test in headless mode") +@click.option('--parallel', is_flag=True, help="Run UI Test in parallel mode") +@click.option('--ci-build-id') @pass_context -def run_ui_tests(context, app, headless=False): +def run_ui_tests(context, app, headless=False, parallel=True, ci_build_id=None): "Run UI tests" site = get_site(context) app_base_path = os.path.abspath(os.path.join(frappe.get_app_path(app), '..')) @@ -598,6 +600,12 @@ def run_ui_tests(context, app, headless=False): command = '{site_env} {password_env} {cypress} {run_or_open}' formatted_command = command.format(site_env=site_env, password_env=password_env, cypress=cypress_path, run_or_open=run_or_open) + if parallel: + formatted_command += ' --parallel' + + if ci_build_id: + formatted_command += ' --ci-build-id {}'.format(ci_build_id) + click.secho("Running Cypress...", fg="yellow") frappe.commands.popen(formatted_command, cwd=app_base_path, raise_err=True) From ad06165ab56f6c08c9fe0c33021d5441b3652f38 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 3 May 2021 00:41:37 +0530 Subject: [PATCH 05/42] fix: Use GITHUB_RUN_ID for uniqueness - Also rename tests --- .github/workflows/server-mariadb-tests.yml | 6 +++--- .github/workflows/server-postgres-tests.yml | 6 +++--- .github/workflows/ui-tests.yml | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index a032025bae..0445420136 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -1,4 +1,4 @@ -name: Server Mariadb tests +name: Server on: pull_request: @@ -13,7 +13,7 @@ jobs: matrix: containers: [1, 2] - name: Server Mariadb + name: MariaDB Tests services: mysql: @@ -94,7 +94,7 @@ jobs: uses: mxschmitt/action-tmate@v3 - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --with-coverage --ci-build-id $GITHUB_RUN_NUMBER + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --with-coverage --ci-build-id $GITHUB_RUN_ID # - name: Coverage - Pull Request # if: github.event_name == 'pull_request' diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index f47268af69..08fb2597c9 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -1,4 +1,4 @@ -name: Server PostgreSQL tests +name: Server on: pull_request: @@ -13,7 +13,7 @@ jobs: matrix: containers: [1, 2] - name: Server PostgreSQL + name: PostgreSQL Tests services: postgres: @@ -94,4 +94,4 @@ jobs: TYPE: server - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --ci-build-id $GITHUB_RUN_NUMBER + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --ci-build-id $GITHUB_RUN_ID diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 705b51c0ed..ea9040e980 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -1,4 +1,4 @@ -name: UI tests +name: UI on: pull_request: @@ -13,7 +13,7 @@ jobs: matrix: containers: [1, 2] - name: UI Mariadb + name: Mariadb Tests services: mysql: @@ -103,4 +103,4 @@ jobs: run: cd ~/frappe-bench/ && bench --site test_site execute frappe.utils.install.complete_setup_wizard - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --headless --parallel --ci-build-id $GITHUB_RUN_NUMBER + run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --headless --parallel --ci-build-id $GITHUB_RUN_ID From e33a09f4e65b672cb4da3dbeaf2392246f448ee0 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 13:15:25 +0530 Subject: [PATCH 06/42] refactor: Test runner - fix style - Handle global dependency --- .github/workflows/ui-tests.yml | 11 ++-- frappe/test_runner.py | 95 +++++++++++++++++++++------------- requirements.txt | 1 + 3 files changed, 64 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index ea9040e980..5c11674293 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -90,17 +90,16 @@ jobs: env: BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} AFTER: ${{ env.GITHUB_EVENT_PATH.after }} - TYPE: 'ui' + TYPE: ui - name: Install run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh env: - DB: 'mariadb' - TYPE: 'ui' + DB: mariadb + TYPE: ui - - name: Run Set-Up - if: matrix.TYPE == 'ui' + - name: Site Setup run: cd ~/frappe-bench/ && bench --site test_site execute frappe.utils.install.complete_setup_wizard - - name: Run Tests + - name: UI Tests run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --headless --parallel --ci-build-id $GITHUB_RUN_ID diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 5207414fe3..15e2fcd45b 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -17,6 +17,9 @@ from six.moves import reload_module from frappe.model.naming import revert_series_if_last import click import requests +import unittest.util + +click.get_current_context().color = True unittest_runner = unittest.TextTestRunner SLOW_TEST_THRESHOLD = 2 @@ -239,6 +242,12 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False): module_name = '{app}.{relative_path}.{module_name}'.format(app=app, relative_path=relative_path.replace('/', '.'), module_name=filename[:-3]) + test_module = importlib.import_module(f'{app}.tests') + + if hasattr(test_module, "global_test_dependencies"): + for doctype in test_module.global_test_dependencies: + make_test_records(doctype, verbose=verbose) + module = importlib.import_module(module_name) if hasattr(module, "test_dependencies"): @@ -425,57 +434,60 @@ def get_test_record_log(): return frappe.flags.test_record_log -class Writeln(object): - def __init__(self,stream): - self.stream = stream - def __getattr__(self, attr): - if attr in ('stream', '__getstate__'): - raise AttributeError(attr) - return getattr(self.stream,attr) - - def writeln(self, arg=None): - if arg: - self.write(arg) - self.write('\n') - -class PrettyPrintResult(unittest.TextTestResult): +class ParallelTestResult(unittest.TextTestResult): def startTest(self, test): super(unittest.TextTestResult, self).startTest(test) - click.echo('\n') + test_class = unittest.util.strclass(test.__class__) + if not hasattr(self, 'current_test_class') or self.current_test_class != test_class: + click.echo(f"\n{unittest.util.strclass(test.__class__)}") + self.current_test_class = test_class + + def getTestMethodName(self, test): + return test._testMethodName if hasattr(test, '_testMethodName') else str(test) def addSuccess(self, test): super(unittest.TextTestResult, self).addSuccess(test) - click.echo("%s %s" % (click.style(' PASS ', bg='green', fg='black'), self.getDescription(test))) + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}") def addError(self, test, err): super(unittest.TextTestResult, self).addError(test, err) - click.echo("%s %s" % (click.style(' ERROR ', bg='red', fg='white'), self.getDescription(test))) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") def addFailure(self, test, err): super(unittest.TextTestResult, self).addFailure(test, err) - click.echo("%s %s" % (click.style(' FAIL ', bg='red', fg='white'), self.getDescription(test))) - click.echo('\n') + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addSkip(self, test, reason): + super(unittest.TextTestResult, self).addSkip(test, reason) + click.echo(f" {click.style(' = ', fg='white')} {self.getTestMethodName(test)}") + + def addExpectedFailure(self, test, err): + super(unittest.TextTestResult, self).addExpectedFailure(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addUnexpectedSuccess(self, test): + super(unittest.TextTestResult, self).addUnexpectedSuccess(test) + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}") def printErrors(self): - if self.dots or self.showAll: - self.stream.writeln() + click.echo('\n') self.printErrorList(' ERROR ', self.errors, 'red') self.printErrorList(' FAIL ', self.failures, 'red') def printErrorList(self, flavour, errors, color): for test, err in errors: click.echo(self.separator1) - click.echo("%s %s" % (click.style(flavour, bg=color), self.getDescription(test))) + click.echo(f"{click.style(flavour, bg=color)} {self.getDescription(test)}") click.echo(self.separator2) - click.echo("%s" % err) + click.echo(err) def __repr__(self): - return f"run={self.testsRun} errors={len(self.errors)} failures={len(self.failures)}" + return f"Tests={self.testsRun} Failing={len(self.failures)} Errors={len(self.errors)}" -def get_all_tests(): +def get_all_tests(app): test_file_list = [] - for path, folders, files in os.walk(frappe.get_pymodule_path('frappe')): + for path, folders, files in os.walk(frappe.get_pymodule_path(app)): for dontwalk in ('locals', '.git', 'public', '__pycache__'): if dontwalk in folders: folders.remove(dontwalk) @@ -486,7 +498,7 @@ def get_all_tests(): # print path for filename in files: - if filename.startswith("test_") and filename.endswith(".py")\ + if filename.startswith("test_") and filename.endswith(".py") \ and filename != 'test_runner.py': test_file_list.append(os.path.join(path, filename)) return test_file_list @@ -495,9 +507,9 @@ class ParallelTestRunner(): def __init__(self, app, site, ci_build_id, ci_instance_id=None, with_coverage=False): self.app = app self.site = site - self.orchestrator_url = 'https://8b52f89a8c13.ngrok.io' - self.ci_build_id = ci_build_id - self.with_coverage = with_coverage + self.orchestrator_url = 'http://1b4f43f01e4d.ngrok.io' + self.ci_build_id = ci_build_id or '123123' + self.with_coverage = False self.setup_test_site() self.ci_instance_id = ci_instance_id or frappe.generate_hash(length=10) frappe.flags.in_test = True @@ -518,7 +530,7 @@ class ParallelTestRunner(): def start_test(self): self.register_instance() - self.test_result = PrettyPrintResult(stream=Writeln(sys.stderr), descriptions=True, verbosity=2) + self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) self.test_status = 'ongoing' self.setup_coverage() @@ -529,8 +541,12 @@ class ParallelTestRunner(): self.call_orchestrator('test-completed') self.submit_coverage() + if self.test_result.failures or self.test_result.errors: + if os.environ.get('CI'): + sys.exit(1) + def register_instance(self): - test_spec_list = get_all_tests() + test_spec_list = get_all_tests(self.app) response_data = self.call_orchestrator('init-test', data={ 'test_spec_list': test_spec_list }) @@ -562,9 +578,13 @@ class ParallelTestRunner(): relative_path=relative_path.replace('/', '.'), module_name=filename[:-3]) module = importlib.import_module(module_name) + frappe.set_user('Administrator') if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype) + try: + make_test_records(doctype) + except: + pass test_suite = unittest.TestSuite() module_test_cases = unittest.TestLoader().loadTestsFromModule(module) @@ -589,7 +609,6 @@ class ParallelTestRunner(): res = requests.post(url, headers=headers, files=files) else: res = requests.get(url, data=data, headers=headers) - print(self.ci_build_id, self.ci_instance_id, endpoint) res.raise_for_status() response_data = {} if 'application/json' in res.headers.get('content-type'): @@ -630,9 +649,11 @@ class ParallelTestRunner(): self.coverage.start() def submit_coverage(self): - if self.with_coverage: - self.coverage.stop() - self.coverage.save() + if not self.with_coverage: + return + + self.coverage.stop() + self.coverage.save() if self.is_master: self.build_coverage_file() diff --git a/requirements.txt b/requirements.txt index 193c5a86b6..98ceaeb202 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,6 +6,7 @@ boto3~=1.17.53 braintree~=4.8.0 chardet~=4.0.0 Click~=7.1.2 +colorama~=0.4.4 coverage~=4.5.4 croniter~=1.0.11 cryptography~=3.4.7 From 57166b9964d51d1be19d5f1097a2060dd880937c Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 13:15:52 +0530 Subject: [PATCH 07/42] test: Fix test dependencies --- .../contacts/doctype/contact/test_contact.py | 5 +-- .../doctype/activity_log/test_activity_log.py | 1 + frappe/core/doctype/file/test_file.py | 1 + .../doctype/role_profile/test_role_profile.py | 4 ++- frappe/tests/__init__.py | 3 ++ frappe/tests/test_email.py | 6 +--- frappe/tests/test_fmt_datetime.py | 32 +------------------ frappe/tests/test_twofactor.py | 2 ++ .../website/doctype/web_form/test_web_form.py | 2 +- 9 files changed, 16 insertions(+), 40 deletions(-) diff --git a/frappe/contacts/doctype/contact/test_contact.py b/frappe/contacts/doctype/contact/test_contact.py index 4929873dc4..b131428696 100644 --- a/frappe/contacts/doctype/contact/test_contact.py +++ b/frappe/contacts/doctype/contact/test_contact.py @@ -5,7 +5,8 @@ from __future__ import unicode_literals import frappe import unittest -from frappe.exceptions import ValidationError + +test_dependencies = ['Contact', 'Salutation'] class TestContact(unittest.TestCase): @@ -52,4 +53,4 @@ def create_contact(name, salutation, emails=None, phones=None, save=True): if save: doc.insert() - return doc \ No newline at end of file + return doc diff --git a/frappe/core/doctype/activity_log/test_activity_log.py b/frappe/core/doctype/activity_log/test_activity_log.py index bd0ea08cc7..8e83b8091d 100644 --- a/frappe/core/doctype/activity_log/test_activity_log.py +++ b/frappe/core/doctype/activity_log/test_activity_log.py @@ -90,4 +90,5 @@ class TestActivityLog(unittest.TestCase): def update_system_settings(args): doc = frappe.get_doc('System Settings') doc.update(args) + doc.flags.ignore_mandatory = 1 doc.save() diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index 2f8f437fc9..2596fe94d0 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -193,6 +193,7 @@ class TestSameContent(unittest.TestCase): class TestFile(unittest.TestCase): def setUp(self): + frappe.set_user('Administrator') self.delete_test_data() self.upload_file() diff --git a/frappe/core/doctype/role_profile/test_role_profile.py b/frappe/core/doctype/role_profile/test_role_profile.py index 624b85c315..975453e8d1 100644 --- a/frappe/core/doctype/role_profile/test_role_profile.py +++ b/frappe/core/doctype/role_profile/test_role_profile.py @@ -5,6 +5,8 @@ from __future__ import unicode_literals import frappe import unittest +test_dependencies = ['Role'] + class TestRoleProfile(unittest.TestCase): def test_make_new_role_profile(self): new_role_profile = frappe.get_doc(dict(doctype='Role Profile', role_profile='Test 1')).insert() @@ -21,4 +23,4 @@ class TestRoleProfile(unittest.TestCase): # clear roles new_role_profile.roles = [] new_role_profile.save() - self.assertEqual(new_role_profile.roles, []) \ No newline at end of file + self.assertEqual(new_role_profile.roles, []) diff --git a/frappe/tests/__init__.py b/frappe/tests/__init__.py index f4dc7f33e2..a310864d83 100644 --- a/frappe/tests/__init__.py +++ b/frappe/tests/__init__.py @@ -3,7 +3,10 @@ import frappe def update_system_settings(args): doc = frappe.get_doc('System Settings') doc.update(args) + doc.flags.ignore_mandatory = 1 doc.save() def get_system_setting(key): return frappe.db.get_single_value("System Settings", key) + +global_test_dependencies = ['User'] diff --git a/frappe/tests/test_email.py b/frappe/tests/test_email.py index ff7e6d534c..af90ee7a6b 100644 --- a/frappe/tests/test_email.py +++ b/frappe/tests/test_email.py @@ -6,11 +6,7 @@ from __future__ import unicode_literals import unittest, frappe, re, email from six import PY3 -from frappe.test_runner import make_test_records - -make_test_records("User") -make_test_records("Email Account") - +test_dependencies = ['Email Account'] class TestEmail(unittest.TestCase): def setUp(self): diff --git a/frappe/tests/test_fmt_datetime.py b/frappe/tests/test_fmt_datetime.py index 20f2af88ba..e19eb25fe6 100644 --- a/frappe/tests/test_fmt_datetime.py +++ b/frappe/tests/test_fmt_datetime.py @@ -45,6 +45,7 @@ class TestFmtDatetime(unittest.TestCase): frappe.db.set_default("time_format", self.pre_test_time_format) frappe.local.user_date_format = None frappe.local.user_time_format = None + frappe.db.rollback() # Test utility functions @@ -97,28 +98,12 @@ class TestFmtDatetime(unittest.TestCase): self.assertEqual(formatdate(test_date), valid_fmt) # Test time formatters - def test_format_time_forced(self): # Test with forced time formats self.assertEqual( format_time(test_time, 'ss:mm:HH'), test_date_obj.strftime('%S:%M:%H')) - @unittest.expectedFailure - def test_format_time_forced_broken_locale(self): - # Test with forced time formats - # Currently format_time defaults to HH:mm:ss if the locale is - # broken, so this is an expected failure. - lang = frappe.local.lang - try: - # Force fallback from Babel - frappe.local.lang = 'FAKE' - self.assertEqual( - format_time(test_time, 'ss:mm:HH'), - test_date_obj.strftime('%S:%M:%H')) - finally: - frappe.local.lang = lang - def test_format_time(self): # Test format_time with various default time formats set for fmt, valid_fmt in test_time_formats.items(): @@ -135,21 +120,6 @@ class TestFmtDatetime(unittest.TestCase): format_datetime(test_datetime, 'dd-yyyy-MM ss:mm:HH'), test_date_obj.strftime('%d-%Y-%m %S:%M:%H')) - @unittest.expectedFailure - def test_format_datetime_forced_broken_locale(self): - # Test with forced datetime formats - # Currently format_datetime defaults to yyyy-MM-dd HH:mm:ss - # if the locale is broken, so this is an expected failure. - lang = frappe.local.lang - # Force fallback from Babel - try: - frappe.local.lang = 'FAKE' - self.assertEqual( - format_datetime(test_datetime, 'dd-yyyy-MM ss:mm:HH'), - test_date_obj.strftime('%d-%Y-%m %S:%M:%H')) - finally: - frappe.local.lang = lang - def test_format_datetime(self): # Test formatdate with various default date formats set for date_fmt, valid_date in test_date_formats.items(): diff --git a/frappe/tests/test_twofactor.py b/frappe/tests/test_twofactor.py index 7acb0a36e8..6c4cfc07c7 100644 --- a/frappe/tests/test_twofactor.py +++ b/frappe/tests/test_twofactor.py @@ -208,12 +208,14 @@ def enable_2fa(bypass_two_factor_auth=0, bypass_restrict_ip_check=0): system_settings.bypass_2fa_for_retricted_ip_users = cint(bypass_two_factor_auth) system_settings.bypass_restrict_ip_check_if_2fa_enabled = cint(bypass_restrict_ip_check) system_settings.two_factor_method = 'OTP App' + system_settings.flags.ignore_mandatory = True system_settings.save(ignore_permissions=True) frappe.db.commit() def disable_2fa(): system_settings = frappe.get_doc('System Settings') system_settings.enable_two_factor_auth = 0 + system_settings.flags.ignore_mandatory = True system_settings.save(ignore_permissions=True) frappe.db.commit() diff --git a/frappe/website/doctype/web_form/test_web_form.py b/frappe/website/doctype/web_form/test_web_form.py index f4a3b5d1e3..99992532d9 100644 --- a/frappe/website/doctype/web_form/test_web_form.py +++ b/frappe/website/doctype/web_form/test_web_form.py @@ -8,7 +8,7 @@ import unittest, json from frappe.website.render import build_page from frappe.website.doctype.web_form.web_form import accept -test_records = frappe.get_test_records('Web Form') +test_dependencies = ['Web Form'] class TestWebForm(unittest.TestCase): def setUp(self): From f6fa55f76c3d82d637f733962d959233d83cf03a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 13:31:02 +0530 Subject: [PATCH 08/42] fix: Run before test hooks --- frappe/test_runner.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 15e2fcd45b..0d82c0de66 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -513,6 +513,7 @@ class ParallelTestRunner(): self.setup_test_site() self.ci_instance_id = ci_instance_id or frappe.generate_hash(length=10) frappe.flags.in_test = True + self.run_before_test_hooks() self.start_test() def setup_test_site(self): @@ -524,7 +525,8 @@ class ParallelTestRunner(): frappe.utils.scheduler.disable_scheduler() set_test_email_config() - def run_before_test_hook(self): + def run_before_test_hooks(self): + click.echo('Running "before_tests" hooks...') for fn in frappe.get_hooks("before_tests", app_name=self.app): frappe.get_attr(fn)() From 0375b9f3afb5e21edd4074cf507aa0c2172063f9 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 14:05:12 +0530 Subject: [PATCH 09/42] chore: Enable coveralls --- .github/workflows/server-mariadb-tests.yml | 39 +++++++++------------- frappe/test_runner.py | 10 +++--- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 0445420136..0b1388fe91 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -96,28 +96,19 @@ jobs: - name: Run Tests run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --with-coverage --ci-build-id $GITHUB_RUN_ID - # - name: Coverage - Pull Request - # if: github.event_name == 'pull_request' - # run: | - # cp ~/frappe-bench/sites/combined_coverage ${GITHUB_WORKSPACE} - # cd ${GITHUB_WORKSPACE} - # if [ -f "./combined_coverage" ]; pip install coveralls==2.2.0 && pip install coverage==4.5.4; fi - # if [ -f "./combined_coverage" ]; coveralls --service=github --source ./combined_coverage; fi - # coveralls --service=github - # env: - # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} - # COVERALLS_SERVICE_NAME: github + - name: Coveralls + uses: coverallsapp/github-action@master + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + flag-name: run-${{ matrix.test_number }} + parallel: true - # - name: Coverage - Push - # if: github.event_name == 'push' - # run: | - # cp ~/frappe-bench/sites/combined_coverage ${GITHUB_WORKSPACE}/.coverage - # cd ${GITHUB_WORKSPACE} - # pip install coveralls==2.2.0 - # pip install coverage==4.5.4 - # coveralls --service=github-actions - # env: - # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} - # COVERALLS_SERVICE_NAME: github-actions + finish: + needs: test + runs-on: ubuntu-18.04 + steps: + - name: Coveralls Finished + uses: coverallsapp/github-action@master + with: + github-token: ${{ secrets.github_token }} + parallel-finished: true diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 0d82c0de66..2452742f6a 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -509,7 +509,7 @@ class ParallelTestRunner(): self.site = site self.orchestrator_url = 'http://1b4f43f01e4d.ngrok.io' self.ci_build_id = ci_build_id or '123123' - self.with_coverage = False + self.with_coverage = with_coverage self.setup_test_site() self.ci_instance_id = ci_instance_id or frappe.generate_hash(length=10) frappe.flags.in_test = True @@ -657,10 +657,10 @@ class ParallelTestRunner(): self.coverage.stop() self.coverage.save() - if self.is_master: - self.build_coverage_file() - else: - self.upload_coverage_file() + # if self.is_master: + # self.build_coverage_file() + # else: + # self.upload_coverage_file() def upload_coverage_file(self): From ead3d76c8458a2d7500b8984199e8ec1994df8c8 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 14:37:12 +0530 Subject: [PATCH 10/42] test: Fix test dependencies --- frappe/core/doctype/docshare/test_docshare.py | 4 +++- frappe/desk/doctype/todo/test_todo.py | 5 ++--- frappe/tests/test_seen.py | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/docshare/test_docshare.py b/frappe/core/doctype/docshare/test_docshare.py index d4ef1f92f8..9c424eb4d7 100644 --- a/frappe/core/doctype/docshare/test_docshare.py +++ b/frappe/core/doctype/docshare/test_docshare.py @@ -7,6 +7,8 @@ import frappe.share import unittest from frappe.automation.doctype.auto_repeat.test_auto_repeat import create_submittable_doctype +test_dependencies = ['User'] + class TestDocShare(unittest.TestCase): def setUp(self): self.user = "test@example.com" @@ -112,4 +114,4 @@ class TestDocShare(unittest.TestCase): self.assertTrue(frappe.has_permission(doctype, "read", doc=submittable_doc.name, user=self.user)) self.assertTrue(frappe.has_permission(doctype, "write", doc=submittable_doc.name, user=self.user)) - frappe.share.remove(doctype, submittable_doc.name, self.user) \ No newline at end of file + frappe.share.remove(doctype, submittable_doc.name, self.user) diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index b767fd4aef..de5b6724a6 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/frappe/desk/doctype/todo/test_todo.py @@ -9,8 +9,7 @@ from frappe.model.db_query import DatabaseQuery from frappe.permissions import add_permission, reset_perms from frappe.core.doctype.doctype.doctype import clear_permissions_cache -# test_records = frappe.get_test_records('ToDo') -test_user_records = frappe.get_test_records('User') +test_dependencies = ['User'] class TestToDo(unittest.TestCase): def test_delete(self): @@ -77,7 +76,7 @@ class TestToDo(unittest.TestCase): frappe.set_user('test4@example.com') #owner and assigned_by is test4 todo3 = create_new_todo('Test3', 'test4@example.com') - + # user without any role to read or write todo document self.assertFalse(todo1.has_permission("read")) self.assertFalse(todo1.has_permission("write")) diff --git a/frappe/tests/test_seen.py b/frappe/tests/test_seen.py index 402bc5f4bf..8eea30d773 100644 --- a/frappe/tests/test_seen.py +++ b/frappe/tests/test_seen.py @@ -41,6 +41,7 @@ class TestSeen(unittest.TestCase): self.assertTrue('test1@example.com' in json.loads(ev._seen)) ev.save() + ev = frappe.get_doc('Event', ev.name) self.assertFalse('test@example.com' in json.loads(ev._seen)) self.assertTrue('test1@example.com' in json.loads(ev._seen)) From bb833799dcbc398e69c215a7ff865e1a0a23a46b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 14:47:41 +0530 Subject: [PATCH 11/42] fix: Click ctx error --- frappe/test_runner.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 2452742f6a..8d6fe48270 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -19,11 +19,13 @@ import click import requests import unittest.util -click.get_current_context().color = True - unittest_runner = unittest.TextTestRunner SLOW_TEST_THRESHOLD = 2 +click_ctx = click.get_current_context(True) +if click_ctx: + click_ctx.color = True + def xmlrunner_wrapper(output): """Convenience wrapper to keep method signature unchanged for XMLTestRunner and TextTestRunner""" def _runner(*args, **kwargs): From 618e23ad90a6e7e7f1e03b837d958d98f41bd994 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 15:45:58 +0530 Subject: [PATCH 12/42] test: Fix test dependencies --- .../doctype/notification/test_notification.py | 4 +--- frappe/tests/test_document.py | 14 ++++---------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/frappe/email/doctype/notification/test_notification.py b/frappe/email/doctype/notification/test_notification.py index 87c4b2527a..31d5d9d1cc 100644 --- a/frappe/email/doctype/notification/test_notification.py +++ b/frappe/email/doctype/notification/test_notification.py @@ -7,9 +7,7 @@ import frappe, frappe.utils, frappe.utils.scheduler from frappe.desk.form import assign_to import unittest -test_records = frappe.get_test_records('Notification') - -test_dependencies = ["User"] +test_dependencies = ["User", "Notification"] class TestNotification(unittest.TestCase): def setUp(self): diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index 2be92be1f5..d0eabfbe8c 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -6,9 +6,8 @@ import os import unittest import frappe -from frappe.utils import cint, add_to_date, now +from frappe.utils import cint from frappe.model.naming import revert_series_if_last, make_autoname, parse_naming_series -from frappe.exceptions import DoesNotExistError class TestDocument(unittest.TestCase): @@ -87,13 +86,13 @@ class TestDocument(unittest.TestCase): d.insert() self.assertEqual(frappe.db.get_value("User", d.name), d.name) - def test_confict_validation(self): + def test_conflict_validation(self): d1 = self.test_insert() d2 = frappe.get_doc(d1.doctype, d1.name) d1.save() self.assertRaises(frappe.TimestampMismatchError, d2.save) - def test_confict_validation_single(self): + def test_conflict_validation_single(self): d1 = frappe.get_doc("Website Settings", "Website Settings") d1.home_page = "test-web-page-1" @@ -110,7 +109,7 @@ class TestDocument(unittest.TestCase): def test_permission_single(self): frappe.set_user("Guest") - d = frappe.get_doc("Website Settings", "Website Settigns") + d = frappe.get_doc("Website Settings", "Website Settings") self.assertRaises(frappe.PermissionError, d.save) frappe.set_user("Administrator") @@ -197,11 +196,6 @@ class TestDocument(unittest.TestCase): self.assertTrue(escaped_xss in d.subject) def test_link_count(self): - if os.environ.get('CI'): - # cannot run this test reliably in travis due to its handling - # of parallelism - return - from frappe.model.utils.link_count import update_link_count update_link_count() From c5910f91eef733c2ee8216847735bdb88521c793 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 16:17:17 +0530 Subject: [PATCH 13/42] ci: Fix coverage file path --- .github/workflows/server-mariadb-tests.yml | 1 + frappe/test_runner.py | 8 +++----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 0b1388fe91..f8147fd14e 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -101,6 +101,7 @@ jobs: with: github-token: ${{ secrets.GITHUB_TOKEN }} flag-name: run-${{ matrix.test_number }} + path-to-cov: ~/frappe-bench/sites/.coverage parallel: true finish: diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 8d6fe48270..08a5e579c2 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -543,7 +543,7 @@ class ParallelTestRunner(): self.print_result() self.call_orchestrator('test-completed') - self.submit_coverage() + self.save_coverage() if self.test_result.failures or self.test_result.errors: if os.environ.get('CI'): @@ -646,13 +646,11 @@ class ParallelTestRunner(): self.coverage = Coverage( source=[source_path], - omit=omit, - data_file='coverage_data', - data_suffix=self.ci_instance_id + omit=omit ) self.coverage.start() - def submit_coverage(self): + def save_coverage(self): if not self.with_coverage: return From d58191346e15a7775442c61151d102465a177e9a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 16:17:50 +0530 Subject: [PATCH 14/42] test: Fix exception & remove unnecessary test --- frappe/tests/test_document.py | 30 ------------------------------ frappe/website/router.py | 2 +- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index d0eabfbe8c..1a5a8721fd 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -195,36 +195,6 @@ class TestDocument(unittest.TestCase): self.assertTrue(xss not in d.subject) self.assertTrue(escaped_xss in d.subject) - def test_link_count(self): - from frappe.model.utils.link_count import update_link_count - - update_link_count() - - doctype, name = 'User', 'test@example.com' - - d = self.test_insert() - d.append('event_participants', {"reference_doctype": doctype, "reference_docname": name}) - - d.save() - - link_count = frappe.cache().get_value('_link_count') or {} - old_count = link_count.get((doctype, name)) or 0 - - frappe.db.commit() - - link_count = frappe.cache().get_value('_link_count') or {} - new_count = link_count.get((doctype, name)) or 0 - - self.assertEqual(old_count + 1, new_count) - - before_update = frappe.db.get_value(doctype, name, 'idx') - - update_link_count() - - after_update = frappe.db.get_value(doctype, name, 'idx') - - self.assertEqual(before_update + new_count, after_update) - def test_naming_series(self): data = ["TEST-", "TEST/17-18/.test_data./.####", "TEST.YYYY.MM.####"] diff --git a/frappe/website/router.py b/frappe/website/router.py index 4acb0c6b9b..b9b4eeb38f 100644 --- a/frappe/website/router.py +++ b/frappe/website/router.py @@ -159,7 +159,7 @@ def evaluate_dynamic_routes(rules, path): endpoint = None if frappe.local.request: - urls = route_map.bind_to_environ(frappe.local.request.environ) + urls = route_map.bind_to_environ(frappe.local.request.environ or dict()) try: endpoint, args = urls.match("/" + path) path = endpoint From 22dfb6a3d883bbc2ffeb73295c0aab59345e27c3 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 17:11:55 +0530 Subject: [PATCH 15/42] ci: Fix path-to-cov --- .github/workflows/server-mariadb-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index f8147fd14e..f79e1333f3 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -101,7 +101,7 @@ jobs: with: github-token: ${{ secrets.GITHUB_TOKEN }} flag-name: run-${{ matrix.test_number }} - path-to-cov: ~/frappe-bench/sites/.coverage + path-to-lcov: ~/frappe-bench/sites/.coverage parallel: true finish: From d0a8f9d01fa58a1d53f54c3306ff6b09fb2f7b0b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 5 May 2021 17:26:32 +0530 Subject: [PATCH 16/42] fix: Handle exception --- frappe/website/router.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/website/router.py b/frappe/website/router.py index b9b4eeb38f..84e29c7a8f 100644 --- a/frappe/website/router.py +++ b/frappe/website/router.py @@ -158,8 +158,8 @@ def evaluate_dynamic_routes(rules, path): route_map = Map(rules) endpoint = None - if frappe.local.request: - urls = route_map.bind_to_environ(frappe.local.request.environ or dict()) + if frappe.local.request and frappe.local.request.environ: + urls = route_map.bind_to_environ(frappe.local.request.environ) try: endpoint, args = urls.match("/" + path) path = endpoint From 88758483553b71ea28da4aec90b28ab5e606d6a0 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 14:49:19 +0530 Subject: [PATCH 17/42] perf(test): Reduce token expiry wait time for test - Also, reduce lock_interval --- frappe/tests/test_auth.py | 2 +- frappe/tests/test_twofactor.py | 10 ++++++---- frappe/twofactor.py | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/frappe/tests/test_auth.py b/frappe/tests/test_auth.py index 086602ea01..bbe9c36aea 100644 --- a/frappe/tests/test_auth.py +++ b/frappe/tests/test_auth.py @@ -110,7 +110,7 @@ class TestLoginAttemptTracker(unittest.TestCase): def test_account_unlock(self): """Make sure that locked account gets unlocked after lock_interval of time. """ - lock_interval = 10 # In sec + lock_interval = 2 # In sec tracker = LoginAttemptTracker(user_name='tester', max_consecutive_login_attempts=1, lock_interval=lock_interval) # Clear the cache by setting attempt as success tracker.add_success_attempt() diff --git a/frappe/tests/test_twofactor.py b/frappe/tests/test_twofactor.py index 6c4cfc07c7..709b88b8f3 100644 --- a/frappe/tests/test_twofactor.py +++ b/frappe/tests/test_twofactor.py @@ -8,7 +8,7 @@ from frappe.utils import cint from frappe.utils import set_request from frappe.auth import validate_ip_address, get_login_attempt_tracker from frappe.twofactor import (should_run_2fa, authenticate_for_2factor, get_cached_user_pass, - two_factor_is_enabled_for_, confirm_otp_token, get_otpsecret_for_, get_verification_obj) + two_factor_is_enabled_for_, confirm_otp_token, get_otpsecret_for_, get_verification_obj, ExpiredLoginException) from . import update_system_settings, get_system_setting import time @@ -111,6 +111,7 @@ class TestTwoFactor(unittest.TestCase): def test_confirm_otp_token(self): '''Ensure otp is confirmed''' + frappe.flags.otp_expiry = 2 authenticate_for_2factor(self.user) tmp_id = frappe.local.response['tmp_id'] otp = 'wrongotp' @@ -118,10 +119,11 @@ class TestTwoFactor(unittest.TestCase): confirm_otp_token(self.login_manager,otp=otp,tmp_id=tmp_id) otp = get_otp(self.user) self.assertTrue(confirm_otp_token(self.login_manager,otp=otp,tmp_id=tmp_id)) + frappe.flags.otp_expiry = None if frappe.flags.tests_verbose: - print('Sleeping for 30secs to confirm token expires..') - time.sleep(30) - with self.assertRaises(frappe.AuthenticationError): + print('Sleeping for 2 secs to confirm token expires..') + time.sleep(2) + with self.assertRaises(ExpiredLoginException): confirm_otp_token(self.login_manager,otp=otp,tmp_id=tmp_id) def test_get_verification_obj(self): diff --git a/frappe/twofactor.py b/frappe/twofactor.py index 0a120d5287..4e098c3075 100644 --- a/frappe/twofactor.py +++ b/frappe/twofactor.py @@ -73,11 +73,11 @@ def cache_2fa_data(user, token, otp_secret, tmp_id): # set increased expiry time for SMS and Email if verification_method in ['SMS', 'Email']: - expiry_time = 300 + expiry_time = frappe.flags.token_expiry or 300 frappe.cache().set(tmp_id + '_token', token) frappe.cache().expire(tmp_id + '_token', expiry_time) else: - expiry_time = 180 + expiry_time = frappe.flags.otp_expiry or 180 for k, v in iteritems({'_usr': user, '_pwd': pwd, '_otp_secret': otp_secret}): frappe.cache().set("{0}{1}".format(tmp_id, k), v) frappe.cache().expire("{0}{1}".format(tmp_id, k), expiry_time) From aaadb71e79493a4975b023203c35ec6c00a44525 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 14:49:40 +0530 Subject: [PATCH 18/42] perf: Make saving of System Settings faster --- frappe/core/doctype/system_settings/system_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/system_settings/system_settings.py b/frappe/core/doctype/system_settings/system_settings.py index d102526a9e..05aaca81de 100644 --- a/frappe/core/doctype/system_settings/system_settings.py +++ b/frappe/core/doctype/system_settings/system_settings.py @@ -42,7 +42,7 @@ class SystemSettings(Document): def on_update(self): for df in self.meta.get("fields"): - if df.fieldtype not in no_value_fields: + if df.fieldtype not in no_value_fields and self.has_value_changed(df.fieldname): frappe.db.set_default(df.fieldname, self.get(df.fieldname)) if self.language: From c1ea512b3aa5c3c4e730dd31a0e855cd111226f4 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 14:51:27 +0530 Subject: [PATCH 19/42] feat: Indicate slow tests --- frappe/test_runner.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 08a5e579c2..6b890b8853 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -244,12 +244,6 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False): module_name = '{app}.{relative_path}.{module_name}'.format(app=app, relative_path=relative_path.replace('/', '.'), module_name=filename[:-3]) - test_module = importlib.import_module(f'{app}.tests') - - if hasattr(test_module, "global_test_dependencies"): - for doctype in test_module.global_test_dependencies: - make_test_records(doctype, verbose=verbose) - module = importlib.import_module(module_name) if hasattr(module, "test_dependencies"): @@ -439,6 +433,7 @@ def get_test_record_log(): class ParallelTestResult(unittest.TextTestResult): def startTest(self, test): + self._started_at = time.time() super(unittest.TextTestResult, self).startTest(test) test_class = unittest.util.strclass(test.__class__) if not hasattr(self, 'current_test_class') or self.current_test_class != test_class: @@ -450,7 +445,10 @@ class ParallelTestResult(unittest.TextTestResult): def addSuccess(self, test): super(unittest.TextTestResult, self).addSuccess(test) - click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}") + elapsed = time.time() - self._started_at + threshold_passed = elapsed >= SLOW_TEST_THRESHOLD + elapsed = click.style(f' ({elapsed:.03}s)', fg='red') if threshold_passed else '' + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}{elapsed}") def addError(self, test, err): super(unittest.TextTestResult, self).addError(test, err) @@ -537,6 +535,8 @@ class ParallelTestRunner(): self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) self.test_status = 'ongoing' + self.make_test_records() + self.setup_coverage() while self.test_status == 'ongoing': self.run_tests_for_file(self.get_next_test()) @@ -561,6 +561,13 @@ class ParallelTestRunner(): self.test_status = response_data.get('status') return response_data.get('next_test') + def make_test_records(self): + test_module = importlib.import_module(f'{self.app}.tests') + + if hasattr(test_module, "global_test_dependencies"): + for doctype in test_module.global_test_dependencies: + make_test_records(doctype) + def run_tests_for_file(self, file_path): if not file_path: return From aab2aca08951584a22ab04cf718faf2861f4e88a Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 14:52:00 +0530 Subject: [PATCH 20/42] ci: Enable coveralls --- .github/workflows/server-mariadb-tests.yml | 60 +++++++++++++++------ .github/workflows/server-postgres-tests.yml | 2 +- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index f79e1333f3..e93474ca6e 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - containers: [1, 2] + container: [1, 2] name: MariaDB Tests @@ -96,20 +96,48 @@ jobs: - name: Run Tests run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --with-coverage --ci-build-id $GITHUB_RUN_ID - - name: Coveralls - uses: coverallsapp/github-action@master - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - flag-name: run-${{ matrix.test_number }} - path-to-lcov: ~/frappe-bench/sites/.coverage - parallel: true + - name: Upload Coverage Data + if: github.event_name == 'pull_request' + run: | + cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE} + cd ${GITHUB_WORKSPACE} + pip install coveralls==2.2.0 + pip install coverage==4.5.4 + coveralls debug --service=github + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} + COVERALLS_FLAG_NAME: run-${{ matrix.test_number }} + COVERALLS_SERVICE_NAME: github + COVERALLS_PARALLEL: true - finish: - needs: test - runs-on: ubuntu-18.04 - steps: + - name: Upload Coverage Data + if: github.event_name == 'push' + run: | + cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE} + cd ${GITHUB_WORKSPACE} + pip install coveralls==2.2.0 + pip install coverage==4.5.4 + coveralls debug --service=github-actions + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} + COVERALLS_FLAG_NAME: run-${{ matrix.container }} + COVERALLS_SERVICE_NAME: github-actions + COVERALLS_PARALLEL: true + + coveralls: + name: Coverage Wrap Up + needs: test + runs-on: ubuntu-18.04 + container: python:3-slim + steps: - name: Coveralls Finished - uses: coverallsapp/github-action@master - with: - github-token: ${{ secrets.github_token }} - parallel-finished: true + run: | + cd ${GITHUB_WORKSPACE} + pip install coveralls==3.0.0 + pip install coverage==4.5.4 + coveralls debug --finish + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index 08fb2597c9..d7697081a2 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -11,7 +11,7 @@ jobs: strategy: fail-fast: false matrix: - containers: [1, 2] + container: [1, 2] name: PostgreSQL Tests From ba88b2cdb9dce5fe31b3db43fa0a29c3abc6ae39 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 14:52:32 +0530 Subject: [PATCH 21/42] fix: Handle exception --- frappe/website/router.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/website/router.py b/frappe/website/router.py index 84e29c7a8f..f3518e179e 100644 --- a/frappe/website/router.py +++ b/frappe/website/router.py @@ -158,7 +158,7 @@ def evaluate_dynamic_routes(rules, path): route_map = Map(rules) endpoint = None - if frappe.local.request and frappe.local.request.environ: + if hasattr(frappe.local, 'request') and frappe.local.request.environ: urls = route_map.bind_to_environ(frappe.local.request.environ) try: endpoint, args = urls.match("/" + path) From b64ca5eb2fc015e4aec31dbc0258820502798876 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 14:52:51 +0530 Subject: [PATCH 22/42] test: Fix dependency --- frappe/website/doctype/blog_post/test_blog_post.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index f0fc484a31..9ecac07ee5 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -13,6 +13,8 @@ from frappe.website.doctype.blog_post.blog_post import get_blog_list from frappe.website.website_generator import WebsiteGenerator from frappe.custom.doctype.customize_form.customize_form import reset_customization +test_dependencies = ['Blog Post'] + class TestBlogPost(unittest.TestCase): def setUp(self): reset_customization('Blog Post') From 3e7b48438ce19ebe216171330b2fc865eccbb6ca Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 15:12:08 +0530 Subject: [PATCH 23/42] chore: Debug --- .github/workflows/server-mariadb-tests.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index e93474ca6e..3ac8f40125 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -130,7 +130,6 @@ jobs: name: Coverage Wrap Up needs: test runs-on: ubuntu-18.04 - container: python:3-slim steps: - name: Coveralls Finished run: | From 5ccccf104d83caf74f13b6ba0593b75bae358301 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Thu, 6 May 2021 17:39:19 +0530 Subject: [PATCH 24/42] chore: Track before test time --- .github/workflows/server-mariadb-tests.yml | 2 +- frappe/test_runner.py | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 3ac8f40125..92d58ee678 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -134,7 +134,7 @@ jobs: - name: Coveralls Finished run: | cd ${GITHUB_WORKSPACE} - pip install coveralls==3.0.0 + pip install coveralls==3.0.1 pip install coverage==4.5.4 coveralls debug --finish env: diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 6b890b8853..3944fbcef5 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -526,17 +526,21 @@ class ParallelTestRunner(): set_test_email_config() def run_before_test_hooks(self): - click.echo('Running "before_tests" hooks...') + start_time = time.time() for fn in frappe.get_hooks("before_tests", app_name=self.app): frappe.get_attr(fn)() + self.make_test_records() + + elapsed = time.time() - start_time + elapsed = click.style(f' ({elapsed:.03}s)', fg='red') + click.echo(f'Before Test {elapsed}') + def start_test(self): self.register_instance() self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) self.test_status = 'ongoing' - self.make_test_records() - self.setup_coverage() while self.test_status == 'ongoing': self.run_tests_for_file(self.get_next_test()) From e737e2ea52308a5c3a6120966880c5378b9ca24a Mon Sep 17 00:00:00 2001 From: Himanshu Date: Fri, 7 May 2021 12:03:58 +0530 Subject: [PATCH 25/42] fix: parse Today default value for date docfield Co-authored-by: Sahil Khan --- frappe/public/js/frappe/form/controls/date.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/date.js b/frappe/public/js/frappe/form/controls/date.js index ca214ca0fa..0b3f7b6479 100644 --- a/frappe/public/js/frappe/form/controls/date.js +++ b/frappe/public/js/frappe/form/controls/date.js @@ -13,9 +13,11 @@ frappe.ui.form.ControlDate = frappe.ui.form.ControlData.extend({ this._super(value); if (this.timepicker_only) return; if (!this.datepicker) return; - if(!value) { + if (!value) { this.datepicker.clear(); return; + } else if (value === "Today") { + value = this.get_now_date(); } let should_refresh = this.last_value && this.last_value !== value; From 2c545496776a5ed64b29918a1ebf74f6b225ef46 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 7 May 2021 21:58:43 +0530 Subject: [PATCH 26/42] feat: Make parallel test independent of test orchestrator --- frappe/commands/utils.py | 7 ++-- frappe/test_runner.py | 83 +++++++++------------------------------- 2 files changed, 22 insertions(+), 68 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index df4250cd10..4d48cac66f 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -554,13 +554,14 @@ def run_tests(context, app=None, module=None, doctype=None, test=(), profile=Fal @click.command('run-parallel-tests') @click.option('--app', help="For App", default='frappe') -@click.option('--ci-build-id', help="CI Build ID") +@click.option('--build-number', help="Build number") +@click.option('--total-builds', help="Total number of builds") @click.option('--with-coverage', is_flag=True, help="Build coverage file") @pass_context -def run_parallel_tests(context, app, ci_build_id, with_coverage): +def run_parallel_tests(context, app, build_number, total_builds, with_coverage): from frappe.test_runner import ParallelTestRunner site = get_site(context) - ParallelTestRunner(app, site=site, ci_build_id=ci_build_id, with_coverage=with_coverage) + ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds, with_coverage=with_coverage) @click.command('run-ui-tests') @click.argument('app') diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 3944fbcef5..cd8bcd6b3c 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -504,14 +504,13 @@ def get_all_tests(app): return test_file_list class ParallelTestRunner(): - def __init__(self, app, site, ci_build_id, ci_instance_id=None, with_coverage=False): + def __init__(self, app, site, build_number=1, total_builds=1, with_coverage=False): self.app = app self.site = site - self.orchestrator_url = 'http://1b4f43f01e4d.ngrok.io' - self.ci_build_id = ci_build_id or '123123' self.with_coverage = with_coverage self.setup_test_site() - self.ci_instance_id = ci_instance_id or frappe.generate_hash(length=10) + self.build_number = frappe.utils.cint(build_number) or 1 + self.total_builds = frappe.utils.cint(total_builds) frappe.flags.in_test = True self.run_before_test_hooks() self.start_test() @@ -537,33 +536,20 @@ class ParallelTestRunner(): def start_test(self): - self.register_instance() self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) self.test_status = 'ongoing' self.setup_coverage() - while self.test_status == 'ongoing': - self.run_tests_for_file(self.get_next_test()) + for test in self.get_test_list(): + self.run_tests_for_file(test) self.print_result() - self.call_orchestrator('test-completed') self.save_coverage() if self.test_result.failures or self.test_result.errors: if os.environ.get('CI'): sys.exit(1) - def register_instance(self): - test_spec_list = get_all_tests(self.app) - response_data = self.call_orchestrator('init-test', data={ - 'test_spec_list': test_spec_list - }) - self.is_master = response_data.get('is_master') - - def get_next_test(self): - response_data = self.call_orchestrator('get-next-test') - self.test_status = response_data.get('status') - return response_data.get('next_test') def make_test_records(self): test_module = importlib.import_module(f'{self.app}.tests') @@ -601,6 +587,14 @@ class ParallelTestRunner(): except: pass + if os.path.basename(os.path.dirname(path)) == "doctype": + txt_file = os.path.join(path, filename[5:].replace(".py", ".json")) + if os.path.exists(txt_file): + with open(txt_file, 'r') as f: + doc = json.loads(f.read()) + doctype = doc["name"] + make_test_records(doctype) + test_suite = unittest.TestSuite() module_test_cases = unittest.TestLoader().loadTestsFromModule(module) test_suite.addTest(module_test_cases) @@ -610,28 +604,6 @@ class ParallelTestRunner(): self.test_result.printErrors() click.echo(self.test_result) - def call_orchestrator(self, endpoint, data={}, files={}): - # add repo token header - # build id in header - headers = { - 'CI-BUILD-ID': self.ci_build_id, - 'CI-INSTANCE-ID': self.ci_instance_id, - 'REPO-TOKEN': '2948288382838DE' - } - url = f'{self.orchestrator_url}/{endpoint}' - - if files: - res = requests.post(url, headers=headers, files=files) - else: - res = requests.get(url, data=data, headers=headers) - res.raise_for_status() - response_data = {} - if 'application/json' in res.headers.get('content-type'): - response_data = res.json() - elif 'application/zip' in res.headers.get('content-type'): - response_data = res.content - - return response_data def setup_coverage(self): if self.with_coverage: @@ -668,29 +640,10 @@ class ParallelTestRunner(): self.coverage.stop() self.coverage.save() - # if self.is_master: - # self.build_coverage_file() - # else: - # self.upload_coverage_file() + def get_test_list(self): + test_list = get_all_tests(self.app) + split_size = frappe.utils.ceil(len(test_list) / self.total_builds) + test_chunks = [test_list[x:x+split_size] for x in range(0, len(test_list), split_size)] + return test_chunks[self.build_number - 1] - def upload_coverage_file(self): - files = {'upload_file': open(f'coverage_data.{self.ci_instance_id}','rb')} - self.call_orchestrator('upload-coverage-file', files=files) - - - def build_coverage_file(self): - import time - import zipfile - import io - click.echo() - while self.call_orchestrator('test-status')['test_status'] == 'ongoing': - click.echo('Waiting for tests to complete...') - time.sleep(5) - - res = self.call_orchestrator('download-coverage-files') - z = zipfile.ZipFile(io.BytesIO(res)) - z.extractall("./coverage_files") - file_list = os.listdir('./coverage_files') - - self.coverage.combine(data_paths=file_list) From 00b6a6729ddc0020b4a6d208609cd0e2ba640bb8 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 7 May 2021 21:59:58 +0530 Subject: [PATCH 27/42] ci: Use right parallel test runner command - Also, fix coverage & coveralls setup --- .github/workflows/server-mariadb-tests.yml | 38 ++++++--------------- .github/workflows/server-postgres-tests.yml | 2 +- requirements.txt | 2 +- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 92d58ee678..97eafcc882 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -89,36 +89,17 @@ jobs: DB: mariadb TYPE: server - - name: Setup tmate session - if: contains(github.event.pull_request.labels.*.name, 'debug-gha') - uses: mxschmitt/action-tmate@v3 - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --with-coverage --ci-build-id $GITHUB_RUN_ID + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --build-number ${{ matrix.container }} --total-builds 2 --with-coverage - name: Upload Coverage Data - if: github.event_name == 'pull_request' run: | cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE} cd ${GITHUB_WORKSPACE} - pip install coveralls==2.2.0 - pip install coverage==4.5.4 - coveralls debug --service=github - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} - COVERALLS_FLAG_NAME: run-${{ matrix.test_number }} - COVERALLS_SERVICE_NAME: github - COVERALLS_PARALLEL: true - - - name: Upload Coverage Data - if: github.event_name == 'push' - run: | - cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE} - cd ${GITHUB_WORKSPACE} - pip install coveralls==2.2.0 - pip install coverage==4.5.4 - coveralls debug --service=github-actions + pip3 install coverage==5.5 + pip3 install coveralls==3.0.1 + coveralls --service=github-actions env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} @@ -129,14 +110,17 @@ jobs: coveralls: name: Coverage Wrap Up needs: test + container: python:3-slim runs-on: ubuntu-18.04 steps: + - name: Clone + uses: actions/checkout@v2 + - name: Coveralls Finished run: | cd ${GITHUB_WORKSPACE} - pip install coveralls==3.0.1 - pip install coverage==4.5.4 - coveralls debug --finish + pip3 install coverage==5.5 + pip3 install coveralls==3.0.1 + coveralls --finish env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index d7697081a2..bef4cbadce 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -94,4 +94,4 @@ jobs: TYPE: server - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --ci-build-id $GITHUB_RUN_ID + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --build-number ${{ matrix.container }} --total-builds 3 diff --git a/requirements.txt b/requirements.txt index 98ceaeb202..769d8c3e7b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ braintree~=4.8.0 chardet~=4.0.0 Click~=7.1.2 colorama~=0.4.4 -coverage~=4.5.4 +coverage==5.5 croniter~=1.0.11 cryptography~=3.4.7 dropbox~=11.7.0 From 13be89f049414693d338142c6d748d88a92f4035 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sat, 8 May 2021 15:45:54 +0530 Subject: [PATCH 28/42] style: Fix sider issues --- frappe/test_runner.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index cd8bcd6b3c..66cf173a21 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -16,7 +16,6 @@ from six import StringIO from six.moves import reload_module from frappe.model.naming import revert_series_if_last import click -import requests import unittest.util unittest_runner = unittest.TextTestRunner @@ -584,7 +583,7 @@ class ParallelTestRunner(): for doctype in module.test_dependencies: try: make_test_records(doctype) - except: + except Exception: pass if os.path.basename(os.path.dirname(path)) == "doctype": From 03d3d67b933b04eb0ffa528777d0d7ef58badf5e Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sat, 8 May 2021 15:49:15 +0530 Subject: [PATCH 29/42] ci: Trial to fix coveralls --- .github/workflows/server-mariadb-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 97eafcc882..3c957eb513 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -99,12 +99,12 @@ jobs: cd ${GITHUB_WORKSPACE} pip3 install coverage==5.5 pip3 install coveralls==3.0.1 - coveralls --service=github-actions + coveralls env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} COVERALLS_FLAG_NAME: run-${{ matrix.container }} - COVERALLS_SERVICE_NAME: github-actions + COVERALLS_SERVICE_NAME: ${{ github.event_name == 'pull_request' && 'github' || 'github-actions' }} COVERALLS_PARALLEL: true coveralls: From 76348b892f4c05c16e6e2590aa763c349e20aeaf Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 9 May 2021 09:32:28 +0530 Subject: [PATCH 30/42] refactor: Move parallel test runner code to a separate file --- frappe/commands/utils.py | 2 +- frappe/parallel_test_runner.py | 212 +++++++++++++++++++++++++++++++ frappe/test_runner.py | 225 --------------------------------- 3 files changed, 213 insertions(+), 226 deletions(-) create mode 100644 frappe/parallel_test_runner.py diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 9062f4811c..f0bae346f5 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -592,7 +592,7 @@ def run_tests(context, app=None, module=None, doctype=None, test=(), profile=Fal @click.option('--with-coverage', is_flag=True, help="Build coverage file") @pass_context def run_parallel_tests(context, app, build_number, total_builds, with_coverage): - from frappe.test_runner import ParallelTestRunner + from frappe.parallel_test_runner import ParallelTestRunner site = get_site(context) ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds, with_coverage=with_coverage) diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py new file mode 100644 index 0000000000..851274d7d4 --- /dev/null +++ b/frappe/parallel_test_runner.py @@ -0,0 +1,212 @@ +import json +import os +import re +import sys +import time +import unittest +import click +import frappe + +from .test_runner import (SLOW_TEST_THRESHOLD, make_test_records, set_test_email_config) + +click_ctx = click.get_current_context(True) +if click_ctx: + click_ctx.color = True + +class ParallelTestRunner(): + def __init__(self, app, site, build_number=1, total_builds=1, with_coverage=False): + self.app = app + self.site = site + self.with_coverage = with_coverage + self.build_number = frappe.utils.cint(build_number) or 1 + self.total_builds = frappe.utils.cint(total_builds) + self.setup_test_site() + self.run_tests() + + def setup_test_site(self): + frappe.init(site=self.site) + if not frappe.db: + frappe.connect() + + frappe.flags.in_test = True + frappe.clear_cache() + frappe.utils.scheduler.disable_scheduler() + set_test_email_config() + self.before_test_setup() + + def before_test_setup(self): + start_time = time.time() + for fn in frappe.get_hooks("before_tests", app_name=self.app): + frappe.get_attr(fn)() + + test_module = frappe.get_module(f'{self.app}.tests') + + if hasattr(test_module, "global_test_dependencies"): + for doctype in test_module.global_test_dependencies: + make_test_records(doctype) + + elapsed = time.time() - start_time + elapsed = click.style(f' ({elapsed:.03}s)', fg='red') + click.echo(f'Before Test {elapsed}') + + def run_tests(self): + self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) + self.test_status = 'ongoing' + + self.start_coverage() + + for test_file_info in self.get_test_file_list(): + self.run_tests_for_file(test_file_info) + + self.save_coverage() + self.print_result() + + def run_tests_for_file(self, file_info): + frappe.set_user('Administrator') + path, filename = file_info + module = self.get_module(path, filename) + self.create_test_dependency_records(module, path, filename) + test_suite = unittest.TestSuite() + module_test_cases = unittest.TestLoader().loadTestsFromModule(module) + test_suite.addTest(module_test_cases) + test_suite(self.test_result) + + def create_test_dependency_records(self, module, path, filename): + if hasattr(module, "test_dependencies"): + for doctype in module.test_dependencies: + make_test_records(doctype) + + if os.path.basename(os.path.dirname(path)) == "doctype": + # test_data_migration_connector.py > data_migration_connector.json + test_record_filename = re.sub('^test_', '', filename).replace(".py", ".json") + test_record_file_path = os.path.join(path, test_record_filename) + if os.path.exists(test_record_file_path): + with open(test_record_file_path, 'r') as f: + doc = json.loads(f.read()) + doctype = doc["name"] + make_test_records(doctype) + + def get_module(self, path, filename): + app_path = frappe.get_pymodule_path(self.app) + relative_path = os.path.relpath(path, app_path) + if relative_path == '.': + module_name = self.app + else: + relative_path = relative_path.replace('/', '.') + module_name = os.path.splitext(filename)[0] + module_name = f'{self.app}.{relative_path}.{module_name}' + + return frappe.get_module(module_name) + + def print_result(self): + self.test_result.printErrors() + click.echo(self.test_result) + if self.test_result.failures or self.test_result.errors: + if os.environ.get('CI'): + sys.exit(1) + + def start_coverage(self): + if self.with_coverage: + from coverage import Coverage + from frappe.utils import get_bench_path + + # Generate coverage report only for app that is being tested + source_path = os.path.join(get_bench_path(), 'apps', self.app) + omit=['*.html', '*.js', '*.xml', '*.css', '*.less', '*.scss', + '*.vue', '*/doctype/*/*_dashboard.py', '*/patches/*'] + + if self.app == 'frappe': + omit.append('*/commands/*') + + self.coverage = Coverage(source=[source_path], omit=omit) + self.coverage.start() + + def save_coverage(self): + if not self.with_coverage: + return + self.coverage.stop() + self.coverage.save() + + def get_test_file_list(self): + test_list = get_all_tests(self.app) + split_size = frappe.utils.ceil(len(test_list) / self.total_builds) + # [1,2,3,4,5,6] to [[1,2], [3,4], [4,6]] if split_size is 2 + test_chunks = [test_list[x:x+split_size] for x in range(0, len(test_list), split_size)] + return test_chunks[self.build_number - 1] + + +class ParallelTestResult(unittest.TextTestResult): + def startTest(self, test): + self._started_at = time.time() + super(unittest.TextTestResult, self).startTest(test) + test_class = unittest.util.strclass(test.__class__) + if not hasattr(self, 'current_test_class') or self.current_test_class != test_class: + click.echo(f"\n{unittest.util.strclass(test.__class__)}") + self.current_test_class = test_class + + def getTestMethodName(self, test): + return test._testMethodName if hasattr(test, '_testMethodName') else str(test) + + def addSuccess(self, test): + super(unittest.TextTestResult, self).addSuccess(test) + elapsed = time.time() - self._started_at + threshold_passed = elapsed >= SLOW_TEST_THRESHOLD + elapsed = click.style(f' ({elapsed:.03}s)', fg='red') if threshold_passed else '' + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}{elapsed}") + + def addError(self, test, err): + super(unittest.TextTestResult, self).addError(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addFailure(self, test, err): + super(unittest.TextTestResult, self).addFailure(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addSkip(self, test, reason): + super(unittest.TextTestResult, self).addSkip(test, reason) + click.echo(f" {click.style(' = ', fg='white')} {self.getTestMethodName(test)}") + + def addExpectedFailure(self, test, err): + super(unittest.TextTestResult, self).addExpectedFailure(test, err) + click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") + + def addUnexpectedSuccess(self, test): + super(unittest.TextTestResult, self).addUnexpectedSuccess(test) + click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}") + + def printErrors(self): + click.echo('\n') + self.printErrorList(' ERROR ', self.errors, 'red') + self.printErrorList(' FAIL ', self.failures, 'red') + + def printErrorList(self, flavour, errors, color): + for test, err in errors: + click.echo(self.separator1) + click.echo(f"{click.style(flavour, bg=color)} {self.getDescription(test)}") + click.echo(self.separator2) + click.echo(err) + + def __str__(self): + return f"Tests: {self.testsRun}, Failing: {len(self.failures)}, Errors: {len(self.errors)}" + +def get_all_tests(app): + test_file_list = [] + for path, folders, files in os.walk(frappe.get_pymodule_path(app)): + for dontwalk in ('locals', '.git', 'public', '__pycache__'): + if dontwalk in folders: + folders.remove(dontwalk) + + # for predictability + folders.sort() + files.sort() + + if os.path.sep.join(["doctype", "doctype", "boilerplate"]) in path: + # in /doctype/doctype/boilerplate/ + continue + + for filename in files: + if filename.startswith("test_") and filename.endswith(".py") \ + and filename != 'test_runner.py': + test_file_list.append([path, filename]) + + return test_file_list diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 66cf173a21..edbe8f4c43 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -9,22 +9,15 @@ import time import xmlrunner import importlib from frappe.modules import load_doctype_module, get_module_name -from frappe.utils import cstr import frappe.utils.scheduler import cProfile, pstats from six import StringIO from six.moves import reload_module from frappe.model.naming import revert_series_if_last -import click -import unittest.util unittest_runner = unittest.TextTestRunner SLOW_TEST_THRESHOLD = 2 -click_ctx = click.get_current_context(True) -if click_ctx: - click_ctx.color = True - def xmlrunner_wrapper(output): """Convenience wrapper to keep method signature unchanged for XMLTestRunner and TextTestRunner""" def _runner(*args, **kwargs): @@ -428,221 +421,3 @@ def get_test_record_log(): frappe.flags.test_record_log = [] return frappe.flags.test_record_log - - -class ParallelTestResult(unittest.TextTestResult): - def startTest(self, test): - self._started_at = time.time() - super(unittest.TextTestResult, self).startTest(test) - test_class = unittest.util.strclass(test.__class__) - if not hasattr(self, 'current_test_class') or self.current_test_class != test_class: - click.echo(f"\n{unittest.util.strclass(test.__class__)}") - self.current_test_class = test_class - - def getTestMethodName(self, test): - return test._testMethodName if hasattr(test, '_testMethodName') else str(test) - - def addSuccess(self, test): - super(unittest.TextTestResult, self).addSuccess(test) - elapsed = time.time() - self._started_at - threshold_passed = elapsed >= SLOW_TEST_THRESHOLD - elapsed = click.style(f' ({elapsed:.03}s)', fg='red') if threshold_passed else '' - click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}{elapsed}") - - def addError(self, test, err): - super(unittest.TextTestResult, self).addError(test, err) - click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") - - def addFailure(self, test, err): - super(unittest.TextTestResult, self).addFailure(test, err) - click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") - - def addSkip(self, test, reason): - super(unittest.TextTestResult, self).addSkip(test, reason) - click.echo(f" {click.style(' = ', fg='white')} {self.getTestMethodName(test)}") - - def addExpectedFailure(self, test, err): - super(unittest.TextTestResult, self).addExpectedFailure(test, err) - click.echo(f" {click.style(' ✖ ', fg='red')} {self.getTestMethodName(test)}") - - def addUnexpectedSuccess(self, test): - super(unittest.TextTestResult, self).addUnexpectedSuccess(test) - click.echo(f" {click.style(' ✔ ', fg='green')} {self.getTestMethodName(test)}") - - def printErrors(self): - click.echo('\n') - self.printErrorList(' ERROR ', self.errors, 'red') - self.printErrorList(' FAIL ', self.failures, 'red') - - def printErrorList(self, flavour, errors, color): - for test, err in errors: - click.echo(self.separator1) - click.echo(f"{click.style(flavour, bg=color)} {self.getDescription(test)}") - click.echo(self.separator2) - click.echo(err) - - def __repr__(self): - return f"Tests={self.testsRun} Failing={len(self.failures)} Errors={len(self.errors)}" - -def get_all_tests(app): - test_file_list = [] - for path, folders, files in os.walk(frappe.get_pymodule_path(app)): - for dontwalk in ('locals', '.git', 'public', '__pycache__'): - if dontwalk in folders: - folders.remove(dontwalk) - - # for predictability - folders.sort() - files.sort() - - # print path - for filename in files: - if filename.startswith("test_") and filename.endswith(".py") \ - and filename != 'test_runner.py': - test_file_list.append(os.path.join(path, filename)) - return test_file_list - -class ParallelTestRunner(): - def __init__(self, app, site, build_number=1, total_builds=1, with_coverage=False): - self.app = app - self.site = site - self.with_coverage = with_coverage - self.setup_test_site() - self.build_number = frappe.utils.cint(build_number) or 1 - self.total_builds = frappe.utils.cint(total_builds) - frappe.flags.in_test = True - self.run_before_test_hooks() - self.start_test() - - def setup_test_site(self): - frappe.init(site=self.site) - if not frappe.db: - frappe.connect() - - frappe.clear_cache() - frappe.utils.scheduler.disable_scheduler() - set_test_email_config() - - def run_before_test_hooks(self): - start_time = time.time() - for fn in frappe.get_hooks("before_tests", app_name=self.app): - frappe.get_attr(fn)() - self.make_test_records() - - elapsed = time.time() - start_time - elapsed = click.style(f' ({elapsed:.03}s)', fg='red') - click.echo(f'Before Test {elapsed}') - - - def start_test(self): - self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) - self.test_status = 'ongoing' - - self.setup_coverage() - for test in self.get_test_list(): - self.run_tests_for_file(test) - - self.print_result() - self.save_coverage() - - if self.test_result.failures or self.test_result.errors: - if os.environ.get('CI'): - sys.exit(1) - - - def make_test_records(self): - test_module = importlib.import_module(f'{self.app}.tests') - - if hasattr(test_module, "global_test_dependencies"): - for doctype in test_module.global_test_dependencies: - make_test_records(doctype) - - def run_tests_for_file(self, file_path): - if not file_path: - return - - app = self.app - filename = file_path.split('/')[-1] - path = file_path.rsplit('/', 1)[0] - - if os.path.sep.join(["doctype", "doctype", "boilerplate"]) in path: - # in /doctype/doctype/boilerplate/ - return - - app_path = frappe.get_pymodule_path(app) - relative_path = os.path.relpath(path, app_path) - if relative_path == '.': - module_name = app - else: - module_name = '{app}.{relative_path}.{module_name}'.format(app=app, - relative_path=relative_path.replace('/', '.'), module_name=filename[:-3]) - - module = importlib.import_module(module_name) - frappe.set_user('Administrator') - if hasattr(module, "test_dependencies"): - for doctype in module.test_dependencies: - try: - make_test_records(doctype) - except Exception: - pass - - if os.path.basename(os.path.dirname(path)) == "doctype": - txt_file = os.path.join(path, filename[5:].replace(".py", ".json")) - if os.path.exists(txt_file): - with open(txt_file, 'r') as f: - doc = json.loads(f.read()) - doctype = doc["name"] - make_test_records(doctype) - - test_suite = unittest.TestSuite() - module_test_cases = unittest.TestLoader().loadTestsFromModule(module) - test_suite.addTest(module_test_cases) - test_suite(self.test_result) - - def print_result(self): - self.test_result.printErrors() - click.echo(self.test_result) - - - def setup_coverage(self): - if self.with_coverage: - from coverage import Coverage - from frappe.utils import get_bench_path - - # Generate coverage report only for app that is being tested - source_path = os.path.join(get_bench_path(), 'apps', self.app) - omit=[ - '*.html', - '*.js', - '*.xml', - '*.css', - '*.less', - '*.scss', - '*.vue', - '*/doctype/*/*_dashboard.py', - '*/patches/*' - ] - - if self.app == 'frappe': - omit.append('*/commands/*') - - self.coverage = Coverage( - source=[source_path], - omit=omit - ) - self.coverage.start() - - def save_coverage(self): - if not self.with_coverage: - return - - self.coverage.stop() - self.coverage.save() - - - def get_test_list(self): - test_list = get_all_tests(self.app) - split_size = frappe.utils.ceil(len(test_list) / self.total_builds) - test_chunks = [test_list[x:x+split_size] for x in range(0, len(test_list), split_size)] - return test_chunks[self.build_number - 1] - From e1fec81ae3f2223de0ff993b612d779620c5a833 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 9 May 2021 11:27:58 +0530 Subject: [PATCH 31/42] feat: Add option to run parallel test with orchestrator --- frappe/commands/utils.py | 15 +++++--- frappe/parallel_test_runner.py | 67 +++++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index f0bae346f5..a0db003ab2 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -587,14 +587,19 @@ def run_tests(context, app=None, module=None, doctype=None, test=(), profile=Fal @click.command('run-parallel-tests') @click.option('--app', help="For App", default='frappe') -@click.option('--build-number', help="Build number") -@click.option('--total-builds', help="Total number of builds") +@click.option('--build-number', help="Build number", default=1) +@click.option('--total-builds', help="Total number of builds", default=1) @click.option('--with-coverage', is_flag=True, help="Build coverage file") +@click.option('--use-orchestrator', is_flag=True, help="Use orchestrator to run parallel tests") @pass_context -def run_parallel_tests(context, app, build_number, total_builds, with_coverage): - from frappe.parallel_test_runner import ParallelTestRunner +def run_parallel_tests(context, app, build_number, total_builds, with_coverage=False, use_orchestrator=False): site = get_site(context) - ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds, with_coverage=with_coverage) + if use_orchestrator: + from frappe.parallel_test_runner import ParallelTestWithOrchestrator + ParallelTestWithOrchestrator(app, site=site, with_coverage=with_coverage) + else: + from frappe.parallel_test_runner import ParallelTestRunner + ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds, with_coverage=with_coverage) @click.command('run-ui-tests') @click.argument('app') diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py index 851274d7d4..ef64ebd8b6 100644 --- a/frappe/parallel_test_runner.py +++ b/frappe/parallel_test_runner.py @@ -6,6 +6,7 @@ import time import unittest import click import frappe +import requests from .test_runner import (SLOW_TEST_THRESHOLD, make_test_records, set_test_email_config) @@ -51,7 +52,6 @@ class ParallelTestRunner(): def run_tests(self): self.test_result = ParallelTestResult(stream=sys.stderr, descriptions=True, verbosity=2) - self.test_status = 'ongoing' self.start_coverage() @@ -62,6 +62,8 @@ class ParallelTestRunner(): self.print_result() def run_tests_for_file(self, file_info): + if not file_info: return + frappe.set_user('Administrator') path, filename = file_info module = self.get_module(path, filename) @@ -210,3 +212,66 @@ def get_all_tests(app): test_file_list.append([path, filename]) return test_file_list + + +class ParallelTestWithOrchestrator(ParallelTestRunner): + ''' + This can be used to balanceout test time across multiple instances + This is dependent on external orchestrator which returns next test to run + + orchestrator endpoints + - register-instance (build_id, instance_id, test_spec_list) + - get-next-test-spec (build_id, instance_id) + - test-completed (build_id, instance_id) + ''' + def __init__(self, app, site, with_coverage=False): + self.orchestrator_url = 'https://test-orchestrator.herokuapp.com' # 'http://localhost:3000' + self.ci_build_id = os.environ.get('CI_BUILD_ID') + self.ci_instance_id = os.environ.get('CI_INSTANCE_ID') or frappe.generate_hash(length=10) + if not self.ci_build_id: + click.echo('CI_BUILD_ID environment variable not found!') + return + + ParallelTestRunner.__init__(self, app, site, with_coverage=with_coverage) + + def run_tests(self): + self.test_status = 'ongoing' + self.register_instance() + super().run_tests() + + def get_test_file_list(self): + while self.test_status == 'ongoing': + yield self.get_next_test() + + def register_instance(self): + test_spec_list = get_all_tests(self.app) + response_data = self.call_orchestrator('register-instance', data={ + 'test_spec_list': test_spec_list + }) + self.is_master = response_data.get('is_master') + + def get_next_test(self): + response_data = self.call_orchestrator('get-next-test-spec') + self.test_status = response_data.get('status') + return response_data.get('next_test') + + def print_result(self): + self.call_orchestrator('test-completed') + return super().print_result() + + def call_orchestrator(self, endpoint, data={}): + # add repo token header + # build id in header + headers = { + 'CI-BUILD-ID': self.ci_build_id, + 'CI-INSTANCE-ID': self.ci_instance_id, + 'REPO-TOKEN': '2948288382838DE' + } + url = f'{self.orchestrator_url}/{endpoint}' + res = requests.get(url, json=data, headers=headers) + res.raise_for_status() + response_data = {} + if 'application/json' in res.headers.get('content-type'): + response_data = res.json() + + return response_data From 3c4306c87aef87f6d64190683ba60767fcce1dee Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 May 2021 09:18:50 +0530 Subject: [PATCH 32/42] refactor: Fix regex to avoid deprecation warnings --- frappe/core/doctype/doctype/doctype.py | 2 +- frappe/core/doctype/doctype/test_doctype.py | 4 ++-- frappe/printing/doctype/print_format/test_print_format.py | 6 +++--- frappe/utils/global_search.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 3588cc553a..abbb4bd191 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -915,7 +915,7 @@ def validate_fields(meta): for field in depends_on_fields: depends_on = docfield.get(field, None) if depends_on and ("=" in depends_on) and \ - re.match("""[\w\.:_]+\s*={1}\s*[\w\.@'"]+""", depends_on): + re.match(r'[\w\.:_]+\s*={1}\s*[\w\.@\'"]+', depends_on): frappe.throw(_("Invalid {0} condition").format(frappe.unscrub(field)), frappe.ValidationError) def check_table_multiselect_option(docfield): diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index bfa9d0ec8a..9c492d2c36 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -92,7 +92,7 @@ class TestDocType(unittest.TestCase): fields=["parent", "depends_on", "collapsible_depends_on", "mandatory_depends_on",\ "read_only_depends_on", "fieldname", "fieldtype"]) - pattern = """[\w\.:_]+\s*={1}\s*[\w\.@'"]+""" + pattern = r'[\w\.:_]+\s*={1}\s*[\w\.@\'"]+' for field in docfields: for depends_on in ["depends_on", "collapsible_depends_on", "mandatory_depends_on", "read_only_depends_on"]: condition = field.get(depends_on) @@ -517,4 +517,4 @@ def new_doctype(name, unique=0, depends_on='', fields=None): for f in fields: doc.append('fields', f) - return doc \ No newline at end of file + return doc diff --git a/frappe/printing/doctype/print_format/test_print_format.py b/frappe/printing/doctype/print_format/test_print_format.py index 7e30bda23e..121916ae5f 100644 --- a/frappe/printing/doctype/print_format/test_print_format.py +++ b/frappe/printing/doctype/print_format/test_print_format.py @@ -12,13 +12,13 @@ class TestPrintFormat(unittest.TestCase): def test_print_user(self, style=None): print_html = frappe.get_print("User", "Administrator", style=style) self.assertTrue("" in print_html) - self.assertTrue(re.findall('
[\s]*administrator[\s]*
', print_html)) + self.assertTrue(re.findall(r'
[\s]*administrator[\s]*
', print_html)) return print_html def test_print_user_standard(self): print_html = self.test_print_user("Standard") - self.assertTrue(re.findall('\.print-format {[\s]*font-size: 9pt;', print_html)) - self.assertFalse(re.findall('th {[\s]*background-color: #eee;[\s]*}', print_html)) + self.assertTrue(re.findall(r'\.print-format {[\s]*font-size: 9pt;', print_html)) + self.assertFalse(re.findall(r'th {[\s]*background-color: #eee;[\s]*}', print_html)) self.assertFalse("font-family: serif;" in print_html) def test_print_user_modern(self): diff --git a/frappe/utils/global_search.py b/frappe/utils/global_search.py index 1c067d0146..c20f3b29d4 100644 --- a/frappe/utils/global_search.py +++ b/frappe/utils/global_search.py @@ -348,7 +348,7 @@ def get_formatted_value(value, field): if getattr(field, 'fieldtype', None) in ["Text", "Text Editor"]: value = unescape_html(frappe.safe_decode(value)) - value = (re.subn(r'<[\s]*(script|style).*?(?s)', '', text_type(value))[0]) + value = (re.subn(r'(?s)<[\s]*(script|style).*?', '', text_type(value))[0]) value = ' '.join(value.split()) return field.label + " : " + strip_html_tags(text_type(value)) From 1829fd3a2b7631cb1cda98ae4fe34323c0bdb935 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 May 2021 12:46:02 +0530 Subject: [PATCH 33/42] ci: Run tests using orchestrator --- .github/workflows/server-mariadb-tests.yml | 4 +++- .github/workflows/server-postgres-tests.yml | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 3c957eb513..2acc517dc5 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -91,7 +91,9 @@ jobs: - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --build-number ${{ matrix.container }} --total-builds 2 --with-coverage + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator --with-coverage + env: + CI_BUILD_ID: ${{ github.run_id }} - name: Upload Coverage Data run: | diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index bef4cbadce..681888c29a 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -94,4 +94,6 @@ jobs: TYPE: server - name: Run Tests - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --build-number ${{ matrix.container }} --total-builds 3 + run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator + env: + CI_BUILD_ID: ${{ github.run_id }} From 0d937cda27ee5a296390cfc8375bda5419198fce Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 May 2021 16:14:36 +0530 Subject: [PATCH 34/42] fix: Get ORCHESTRATOR_URL from secrets --- .github/workflows/server-mariadb-tests.yml | 1 + .github/workflows/server-postgres-tests.yml | 1 + frappe/parallel_test_runner.py | 17 +++++++++++------ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 2acc517dc5..1c0d7324cc 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -94,6 +94,7 @@ jobs: run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator --with-coverage env: CI_BUILD_ID: ${{ github.run_id }} + ORCHESTRATOR_URL: ${{ secrets.ORCHESTRATOR_URL }} - name: Upload Coverage Data run: | diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index 681888c29a..e11433311c 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -97,3 +97,4 @@ jobs: run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator env: CI_BUILD_ID: ${{ github.run_id }} + ORCHESTRATOR_URL: ${{ secrets.ORCHESTRATOR_URL }} diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py index ef64ebd8b6..1dbb24f191 100644 --- a/frappe/parallel_test_runner.py +++ b/frappe/parallel_test_runner.py @@ -216,21 +216,26 @@ def get_all_tests(app): class ParallelTestWithOrchestrator(ParallelTestRunner): ''' - This can be used to balanceout test time across multiple instances + This can be used to balance-out test time across multiple instances This is dependent on external orchestrator which returns next test to run orchestrator endpoints - - register-instance (build_id, instance_id, test_spec_list) - - get-next-test-spec (build_id, instance_id) - - test-completed (build_id, instance_id) + - register-instance (, , test_spec_list) + - get-next-test-spec (, ) + - test-completed (, ) ''' def __init__(self, app, site, with_coverage=False): - self.orchestrator_url = 'https://test-orchestrator.herokuapp.com' # 'http://localhost:3000' + self.orchestrator_url = os.environ.get('ORCHESTRATOR_URL') + if not self.orchestrator_url: + click.echo('ORCHESTRATOR_URL environment variable not found!') + click.echo('Pass public URL after hosting https://github.com/frappe/test-orchestrator') + sys.exit(1) + self.ci_build_id = os.environ.get('CI_BUILD_ID') self.ci_instance_id = os.environ.get('CI_INSTANCE_ID') or frappe.generate_hash(length=10) if not self.ci_build_id: click.echo('CI_BUILD_ID environment variable not found!') - return + sys.exit(1) ParallelTestRunner.__init__(self, app, site, with_coverage=with_coverage) From 803741466df733d0d1b2a278eb9f61405ff78855 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 May 2021 18:38:36 +0530 Subject: [PATCH 35/42] fix: Sort dependency options for predictability --- frappe/test_runner.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/test_runner.py b/frappe/test_runner.py index edbe8f4c43..cd71dd46c5 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -307,6 +307,8 @@ def get_dependencies(doctype): if doctype_name in options_list: options_list.remove(doctype_name) + options_list.sort() + return options_list def make_test_records_for_doctype(doctype, verbose=0, force=False): From ed2a3aa5895abaacd82449140811520310ee78ea Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 May 2021 23:48:32 +0530 Subject: [PATCH 36/42] ci: Add test orchestrator URL --- .github/workflows/server-mariadb-tests.yml | 2 +- .github/workflows/server-postgres-tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 1c0d7324cc..93a9317ad0 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -94,7 +94,7 @@ jobs: run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator --with-coverage env: CI_BUILD_ID: ${{ github.run_id }} - ORCHESTRATOR_URL: ${{ secrets.ORCHESTRATOR_URL }} + ORCHESTRATOR_URL: http://test-orchestrator.frappe.io - name: Upload Coverage Data run: | diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index e11433311c..5d6b4b87dc 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -97,4 +97,4 @@ jobs: run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator env: CI_BUILD_ID: ${{ github.run_id }} - ORCHESTRATOR_URL: ${{ secrets.ORCHESTRATOR_URL }} + ORCHESTRATOR_URL: http://test-orchestrator.frappe.io From be9b18191be5672d0266c0df7f2f49b39c5ea2d9 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 11 May 2021 09:50:58 +0530 Subject: [PATCH 37/42] refactor: Resolve deprecation warnings --- frappe/core/doctype/data_export/exporter.py | 2 +- frappe/patches/v5_0/fix_text_editor_file_urls.py | 3 +-- frappe/utils/formatters.py | 2 +- frappe/website/doctype/web_page/test_web_page.py | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py index bec8cde7ea..5d600cc0db 100644 --- a/frappe/core/doctype/data_export/exporter.py +++ b/frappe/core/doctype/data_export/exporter.py @@ -282,7 +282,7 @@ class DataExporter: try: sflags = self.docs_to_export.get("flags", "I,U").upper() flags = 0 - for a in re.split('\W+',sflags): + for a in re.split(r'\W+', sflags): flags = flags | reflags.get(a,0) c = re.compile(names, flags) diff --git a/frappe/patches/v5_0/fix_text_editor_file_urls.py b/frappe/patches/v5_0/fix_text_editor_file_urls.py index d91aad0234..a6d7d2fb9a 100644 --- a/frappe/patches/v5_0/fix_text_editor_file_urls.py +++ b/frappe/patches/v5_0/fix_text_editor_file_urls.py @@ -33,8 +33,7 @@ def execute(): def scrub_relative_urls(html): """prepend a slash before a relative url""" try: - return re.sub("""src[\s]*=[\s]*['"]files/([^'"]*)['"]""", 'src="/files/\g<1>"', html) - # return re.sub("""(src|href)[^\w'"]*['"](?!http|ftp|mailto|/|#|%|{|cid:|\.com/www\.)([^'" >]+)['"]""", '\g<1>="/\g<2>"', html) + return re.sub(r'src[\s]*=[\s]*[\'"]files/([^\'"]*)[\'"]', r'src="/files/\g<1>"', html) except: print("Error", html) raise diff --git a/frappe/utils/formatters.py b/frappe/utils/formatters.py index 5d1e9bdb19..7913413878 100644 --- a/frappe/utils/formatters.py +++ b/frappe/utils/formatters.py @@ -78,7 +78,7 @@ def format_value(value, df=None, doc=None, currency=None, translated=False): return "{}%".format(flt(value, 2)) elif df.get("fieldtype") in ("Text", "Small Text"): - if not re.search("(\") elif df.get("fieldtype") == "Markdown Editor": diff --git a/frappe/website/doctype/web_page/test_web_page.py b/frappe/website/doctype/web_page/test_web_page.py index a481337978..7a2ddc6961 100644 --- a/frappe/website/doctype/web_page/test_web_page.py +++ b/frappe/website/doctype/web_page/test_web_page.py @@ -39,7 +39,7 @@ class TestWebPage(unittest.TestCase): published = 1, content_type = 'Rich Text', main_section = 'rich text', - main_section_md = '# h1\n\markdown content', + main_section_md = '# h1\nmarkdown content', main_section_html = '
html content
' )).insert() From 17928f2d280ae09f8cec197647e606eb583e253e Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 11 May 2021 15:30:55 +0530 Subject: [PATCH 38/42] fix: Clear web page cache after web template is updated --- .../website/doctype/web_template/web_template.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/frappe/website/doctype/web_template/web_template.py b/frappe/website/doctype/web_template/web_template.py index 3c61807099..2fd5bfa179 100644 --- a/frappe/website/doctype/web_template/web_template.py +++ b/frappe/website/doctype/web_template/web_template.py @@ -9,6 +9,7 @@ from shutil import rmtree import frappe from frappe.model.document import Document +from frappe.website.render import clear_cache from frappe import _ from frappe.modules.export_file import ( write_document_file, @@ -37,6 +38,19 @@ class WebTemplate(Document): if was_standard and not self.standard: self.import_from_files() + def on_update(self): + """Clear cache for all Web Pages in which this template is used""" + routes = frappe.db.get_all( + "Web Page", + filters=[ + ["Web Page Block", "web_template", "=", self.name], + ["Web Page", "published", "=", 1], + ], + pluck="route", + ) + for route in routes: + clear_cache(route) + def on_trash(self): if frappe.conf.developer_mode and self.standard: # delete template html and json files From 095994a86c0fcebc0d72ff2a18460efac7e9a0ff Mon Sep 17 00:00:00 2001 From: casesolved-co-uk Date: Tue, 11 May 2021 10:14:56 +0000 Subject: [PATCH 39/42] fix: connected app auto_refresh credentials and mandatory fields --- .../doctype/connected_app/connected_app.json | 11 +++++++---- .../doctype/connected_app/connected_app.py | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/frappe/integrations/doctype/connected_app/connected_app.json b/frappe/integrations/doctype/connected_app/connected_app.json index e5dbb0472a..b5330f4d4f 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.json +++ b/frappe/integrations/doctype/connected_app/connected_app.json @@ -54,7 +54,8 @@ "fieldname": "client_id", "fieldtype": "Data", "in_list_view": 1, - "label": "Client Id" + "label": "Client Id", + "mandatory_depends_on": "eval:doc.redirect_uri" }, { "fieldname": "redirect_uri", @@ -96,12 +97,14 @@ { "fieldname": "authorization_uri", "fieldtype": "Data", - "label": "Authorization URI" + "label": "Authorization URI", + "mandatory_depends_on": "eval:doc.redirect_uri" }, { "fieldname": "token_uri", "fieldtype": "Data", - "label": "Token URI" + "label": "Token URI", + "mandatory_depends_on": "eval:doc.redirect_uri" }, { "fieldname": "revocation_uri", @@ -136,7 +139,7 @@ "link_fieldname": "connected_app" } ], - "modified": "2020-11-16 16:29:50.277405", + "modified": "2021-05-10 05:03:06.296863", "modified_by": "Administrator", "module": "Integrations", "name": "Connected App", diff --git a/frappe/integrations/doctype/connected_app/connected_app.py b/frappe/integrations/doctype/connected_app/connected_app.py index 95077ece77..449e30f6d0 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.py +++ b/frappe/integrations/doctype/connected_app/connected_app.py @@ -26,20 +26,27 @@ class ConnectedApp(Document): self.redirect_uri = urljoin(base_url, callback_path) def get_oauth2_session(self, user=None, init=False): + """Return an auto-refreshing OAuth2 session which is an extension of a requests.Session()""" token = None token_updater = None + auto_refresh_kwargs = None if not init: user = user or frappe.session.user token_cache = self.get_user_token(user) token = token_cache.get_json() token_updater = token_cache.update_data + auto_refresh_kwargs = {'client_id': self.client_id} + client_secret = self.get_password('client_secret') + if client_secret: + auto_refresh_kwargs['client_secret'] = client_secret return OAuth2Session( client_id=self.client_id, token=token, token_updater=token_updater, auto_refresh_url=self.token_uri, + auto_refresh_kwargs=auto_refresh_kwargs, redirect_uri=self.redirect_uri, scope=self.get_scopes() ) From 155b28cf518d755c40f5bebc8cd8dcd062e0736f Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 12 May 2021 16:29:05 +0530 Subject: [PATCH 40/42] ci: Update Job names - Also, update mergify rules --- .github/workflows/server-mariadb-tests.yml | 2 +- .github/workflows/server-postgres-tests.yml | 2 +- .github/workflows/ui-tests.yml | 2 +- .mergify.yml | 22 ++++++++++----------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 93a9317ad0..075b76e8a1 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -13,7 +13,7 @@ jobs: matrix: container: [1, 2] - name: MariaDB Tests + name: Python Unit Tests (MariaDB) services: mysql: diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index 5d6b4b87dc..4325eebaad 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -13,7 +13,7 @@ jobs: matrix: container: [1, 2] - name: PostgreSQL Tests + name: Python Unit Tests (Postgres) services: postgres: diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 5c11674293..9eea128cd1 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -13,7 +13,7 @@ jobs: matrix: containers: [1, 2] - name: Mariadb Tests + name: UI Tests (Cypress) services: mysql: diff --git a/.mergify.yml b/.mergify.yml index 82f710a5a8..6535c5eb48 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -1,12 +1,12 @@ pull_request_rules: - name: Automatic merge on CI success and review conditions: - - status-success=Sider - - status-success=Semantic Pull Request - - status-success=Python MariaDB - - status-success=Python PostgreSQL - - status-success=UI MariaDB - - status-success=security/snyk (frappe) + - check-success=Sider + - check-success=Semantic Pull Request + - check-success=Python Unit Tests (MariaDB) + - check-success=Python Unit Tests (Postgres) + - check-success=UI Tests (Cypress) + - check-success=security/snyk (frappe) - label!=dont-merge - label!=squash - "#approved-reviews-by>=1" @@ -15,11 +15,11 @@ pull_request_rules: method: merge - name: Automatic squash on CI success and review conditions: - - status-success=Sider - - status-success=Python MariaDB - - status-success=Python PostgreSQL - - status-success=UI MariaDB - - status-success=security/snyk (frappe) + - check-success=Sider + - check-success=Python Unit Tests (MariaDB) + - check-success=Python Unit Tests (Postgres) + - check-success=UI Tests (Cypress) + - check-success=security/snyk (frappe) - label!=dont-merge - label=squash - "#approved-reviews-by>=1" From a34a278e06de67a5f48f50217bcbc84e03f95e69 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 12 May 2021 17:57:37 +0530 Subject: [PATCH 41/42] chore: Update .mergify.yml --- .mergify.yml | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index 6535c5eb48..c759c1e3ec 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -1,12 +1,15 @@ pull_request_rules: - name: Automatic merge on CI success and review conditions: - - check-success=Sider - - check-success=Semantic Pull Request - - check-success=Python Unit Tests (MariaDB) - - check-success=Python Unit Tests (Postgres) - - check-success=UI Tests (Cypress) - - check-success=security/snyk (frappe) + - status-success=Sider + - status-success=Semantic Pull Request + - status-success=Python Unit Tests (MariaDB) (1) + - status-success=Python Unit Tests (MariaDB) (2) + - status-success=Python Unit Tests (Postgres) (1) + - status-success=Python Unit Tests (Postgres) (2) + - status-success=UI Tests (Cypress) (1) + - status-success=UI Tests (Cypress) (2) + - status-success=security/snyk (frappe) - label!=dont-merge - label!=squash - "#approved-reviews-by>=1" @@ -15,11 +18,14 @@ pull_request_rules: method: merge - name: Automatic squash on CI success and review conditions: - - check-success=Sider - - check-success=Python Unit Tests (MariaDB) - - check-success=Python Unit Tests (Postgres) - - check-success=UI Tests (Cypress) - - check-success=security/snyk (frappe) + - status-success=Sider + - status-success=Python Unit Tests (MariaDB) (1) + - status-success=Python Unit Tests (MariaDB) (2) + - status-success=Python Unit Tests (Postgres) (1) + - status-success=Python Unit Tests (Postgres) (2) + - status-success=UI Tests (Cypress) (1) + - status-success=UI Tests (Cypress) (2) + - status-success=security/snyk (frappe) - label!=dont-merge - label=squash - "#approved-reviews-by>=1" From 4d7f5a8f8dd2b14f1bad026ad1283402ab8bb541 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 12 May 2021 20:22:48 +0530 Subject: [PATCH 42/42] ci: fix semgrep false positives (#13161) * ci: fix false positive rule for split js translate - limit regex to must match end of line. - expand previous check to take care of other ways to split multi-line calls. * ci: update tests for rules, ignore rules in sider * ci: enable semgrep on v13 branches * ci: fix false positive for python split strings --- .flake8 | 3 +- .../semgrep_rules/frappe_correctness.py | 66 ++++++++++++++----- .github/helper/semgrep_rules/translate.js | 7 ++ .github/helper/semgrep_rules/translate.py | 8 +++ .github/helper/semgrep_rules/translate.yml | 8 +-- .github/workflows/semgrep.yml | 2 + 6 files changed, 74 insertions(+), 20 deletions(-) diff --git a/.flake8 b/.flake8 index 399b176e1d..56c9b9a369 100644 --- a/.flake8 +++ b/.flake8 @@ -29,4 +29,5 @@ ignore = B950, W191, -max-line-length = 200 \ No newline at end of file +max-line-length = 200 +exclude=.github/helper/semgrep_rules diff --git a/.github/helper/semgrep_rules/frappe_correctness.py b/.github/helper/semgrep_rules/frappe_correctness.py index 37889fbbb1..745e6463b8 100644 --- a/.github/helper/semgrep_rules/frappe_correctness.py +++ b/.github/helper/semgrep_rules/frappe_correctness.py @@ -4,25 +4,61 @@ from frappe import _, flt from frappe.model.document import Document +# ruleid: frappe-modifying-but-not-comitting def on_submit(self): if self.value_of_goods == 0: frappe.throw(_('Value of goods cannot be 0')) - # ruleid: frappe-modifying-after-submit self.status = 'Submitted' -def on_submit(self): # noqa - if flt(self.per_billed) < 100: - self.update_billing_status() - else: - # todook: frappe-modifying-after-submit - self.status = "Completed" - self.db_set("status", "Completed") -class TestDoc(Document): - pass +# ok: frappe-modifying-but-not-comitting +def on_submit(self): + if self.value_of_goods == 0: + frappe.throw(_('Value of goods cannot be 0')) + self.status = 'Submitted' + self.db_set('status', 'Submitted') - def validate(self): - #ruleid: frappe-modifying-child-tables-while-iterating - for item in self.child_table: - if item.value < 0: - self.remove(item) +# ok: frappe-modifying-but-not-comitting +def on_submit(self): + if self.value_of_goods == 0: + frappe.throw(_('Value of goods cannot be 0')) + x = "y" + self.status = x + self.db_set('status', x) + + +# ok: frappe-modifying-but-not-comitting +def on_submit(self): + x = "y" + self.status = x + self.save() + +# ruleid: frappe-modifying-but-not-comitting-other-method +class DoctypeClass(Document): + def on_submit(self): + self.good_method() + self.tainted_method() + + def tainted_method(self): + self.status = "uptate" + + +# ok: frappe-modifying-but-not-comitting-other-method +class DoctypeClass(Document): + def on_submit(self): + self.good_method() + self.tainted_method() + + def tainted_method(self): + self.status = "update" + self.db_set("status", "update") + +# ok: frappe-modifying-but-not-comitting-other-method +class DoctypeClass(Document): + def on_submit(self): + self.good_method() + self.tainted_method() + self.save() + + def tainted_method(self): + self.status = "uptate" diff --git a/.github/helper/semgrep_rules/translate.js b/.github/helper/semgrep_rules/translate.js index 7b92fe2dff..9cdfb75d0b 100644 --- a/.github/helper/semgrep_rules/translate.js +++ b/.github/helper/semgrep_rules/translate.js @@ -35,3 +35,10 @@ __('You have' + 'subscribers in your mailing list.') // ruleid: frappe-translation-js-splitting __('You have {0} subscribers' + 'in your mailing list', [subscribers.length]) + +// ok: frappe-translation-js-splitting +__("Ctrl+Enter to add comment") + +// ruleid: frappe-translation-js-splitting +__('You have {0} subscribers \ + in your mailing list', [subscribers.length]) diff --git a/.github/helper/semgrep_rules/translate.py b/.github/helper/semgrep_rules/translate.py index bd6cd9126c..9de6aa94f0 100644 --- a/.github/helper/semgrep_rules/translate.py +++ b/.github/helper/semgrep_rules/translate.py @@ -51,3 +51,11 @@ _(f"what" + f"this is also not cool") _("") # ruleid: frappe-translation-empty-string _('') + + +class Test: + # ok: frappe-translation-python-splitting + def __init__( + args + ): + pass diff --git a/.github/helper/semgrep_rules/translate.yml b/.github/helper/semgrep_rules/translate.yml index 7754b52efc..5f03fb9fd0 100644 --- a/.github/helper/semgrep_rules/translate.yml +++ b/.github/helper/semgrep_rules/translate.yml @@ -44,8 +44,8 @@ rules: pattern-either: - pattern: _(...) + _(...) - pattern: _("..." + "...") - - pattern-regex: '_\([^\)]*\\\s*' # lines broken by `\` - - pattern-regex: '_\(\s*\n' # line breaks allowed by python for using ( ) + - pattern-regex: '[\s\.]_\([^\)]*\\\s*' # lines broken by `\` + - pattern-regex: '[\s\.]_\(\s*\n' # line breaks allowed by python for using ( ) message: | Do not split strings inside translate function. Do not concatenate using translate functions. Please refer: https://frappeframework.com/docs/user/en/translations @@ -54,8 +54,8 @@ rules: - id: frappe-translation-js-splitting pattern-either: - - pattern-regex: '__\([^\)]*[\+\\]\s*' - - pattern: __('...' + '...') + - pattern-regex: '__\([^\)]*[\\]\s+' + - pattern: __('...' + '...', ...) - pattern: __('...') + __('...') message: | Do not split strings inside translate function. Do not concatenate using translate functions. diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index 5092bf4705..389524e968 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -4,6 +4,8 @@ on: pull_request: branches: - develop + - version-13-hotfix + - version-13-pre-release jobs: semgrep: name: Frappe Linter