From ca515e55837b076c3b3cc5fa57d3fdb0504242ef Mon Sep 17 00:00:00 2001 From: m-e-l-u-h-a-n Date: Wed, 3 Mar 2021 09:30:15 +0530 Subject: [PATCH] tools: Move registration of --force to test-scripts.py. This commit moves --force option used with various tests to test-scripts.py to have it alongside the logic that does provisioning status assertion. This is a step towards providing more clarity over use of this argument with tests as asked in issue #17455. --- tools/check-provision | 5 +++-- tools/lib/test_script.py | 8 ++++++++ tools/lint | 17 ++++++++++------- tools/run-dev.py | 4 ++-- tools/run-mypy | 6 ++---- tools/test-api | 6 ++---- tools/test-backend | 7 +++++-- tools/test-help-documentation | 9 ++++++--- tools/test-js-with-node | 6 +++--- tools/test-js-with-puppeteer | 20 +++++++++++--------- 10 files changed, 52 insertions(+), 36 deletions(-) diff --git a/tools/check-provision b/tools/check-provision index 952f40b076..1ebab34b80 100755 --- a/tools/check-provision +++ b/tools/check-provision @@ -7,12 +7,13 @@ tools_dir = os.path.dirname(os.path.abspath(__file__)) root_dir = os.path.dirname(tools_dir) sys.path.insert(0, root_dir) -from tools.lib.test_script import assert_provisioning_status_ok +from tools.lib.test_script import add_provision_check_override_param, assert_provisioning_status_ok def run() -> None: parser = argparse.ArgumentParser() - parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") + add_provision_check_override_param(parser) + options = parser.parse_args() assert_provisioning_status_ok(options.force) diff --git a/tools/lib/test_script.py b/tools/lib/test_script.py index e8789a4e08..e1b346b071 100644 --- a/tools/lib/test_script.py +++ b/tools/lib/test_script.py @@ -2,6 +2,7 @@ import glob import os import subprocess import sys +from argparse import ArgumentParser from distutils.version import LooseVersion from typing import Iterable, List, Optional, Tuple @@ -88,6 +89,13 @@ def assert_provisioning_status_ok(force: bool) -> None: sys.exit(1) +def add_provision_check_override_param(parser: ArgumentParser) -> None: + """ + Registers --force argument to be used with various commands/tests in our tools. + """ + parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") + + def find_js_test_files(test_dir: str, files: Iterable[str]) -> List[str]: test_files = [] for file in files: diff --git a/tools/lint b/tools/lint index 6f982c05f9..6b018e7376 100755 --- a/tools/lint +++ b/tools/lint @@ -16,21 +16,24 @@ from linter_lib.custom_check import non_py_rules, python_rules def run() -> None: - parser = argparse.ArgumentParser() - parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") - parser.add_argument("--full", action="store_true", help="Check some things we typically ignore") - add_default_linter_arguments(parser) - args = parser.parse_args() - tools_dir = os.path.dirname(os.path.abspath(__file__)) root_dir = os.path.dirname(tools_dir) sys.path.insert(0, root_dir) - from tools.lib.test_script import assert_provisioning_status_ok + from tools.lib.test_script import ( + add_provision_check_override_param, + assert_provisioning_status_ok, + ) from tools.linter_lib.exclude import EXCLUDED_FILES, PUPPET_CHECK_RULES_TO_EXCLUDE from tools.linter_lib.pep8 import check_pep8 from tools.linter_lib.pyflakes import check_pyflakes + parser = argparse.ArgumentParser() + add_provision_check_override_param(parser) + parser.add_argument("--full", action="store_true", help="Check some things we typically ignore") + add_default_linter_arguments(parser) + args = parser.parse_args() + os.chdir(root_dir) assert_provisioning_status_ok(args.force) diff --git a/tools/run-dev.py b/tools/run-dev.py index faf1772173..cc2c155671 100755 --- a/tools/run-dev.py +++ b/tools/run-dev.py @@ -19,7 +19,7 @@ from tornado.ioloop import IOLoop TOOLS_DIR = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.dirname(TOOLS_DIR)) -from tools.lib.test_script import assert_provisioning_status_ok +from tools.lib.test_script import add_provision_check_override_param, assert_provisioning_status_ok if "posix" in os.name and os.geteuid() == 0: raise RuntimeError("run-dev.py should not be run as root.") @@ -51,12 +51,12 @@ parser.add_argument( help="Do not clear memcached on startup", ) parser.add_argument("--streamlined", action="store_true", help="Avoid thumbor, etc.") -parser.add_argument("--force", action="store_true", help="Run command despite possible problems.") parser.add_argument( "--enable-tornado-logging", action="store_true", help="Enable access logs from tornado proxy server.", ) +add_provision_check_override_param(parser) options = parser.parse_args() assert_provisioning_status_ok(options.force) diff --git a/tools/run-mypy b/tools/run-mypy index 8af03d103c..56ef94ee0a 100755 --- a/tools/run-mypy +++ b/tools/run-mypy @@ -11,7 +11,7 @@ TOOLS_DIR = os.path.dirname(os.path.abspath(__file__)) os.chdir(os.path.dirname(TOOLS_DIR)) sys.path.append(os.path.dirname(TOOLS_DIR)) -from lib.test_script import assert_provisioning_status_ok +from tools.lib.test_script import add_provision_check_override_param, assert_provisioning_status_ok exclude = """ stubs/ @@ -27,9 +27,7 @@ parser.add_argument( parser.add_argument( "-a", "--all", action="store_true", help="check all files, bypassing the default exclude list" ) -parser.add_argument( - "--force", action="store_true", help="run tests despite possible provisioning problems" -) +add_provision_check_override_param(parser) parser.add_argument("--quiet", action="store_true", help="suppress mypy summary output") args = parser.parse_args() diff --git a/tools/test-api b/tools/test-api index 0af88172cd..c27e15e9db 100755 --- a/tools/test-api +++ b/tools/test-api @@ -16,14 +16,12 @@ os.chdir(ZULIP_PATH) from zulip import Client -from tools.lib.test_script import assert_provisioning_status_ok +from tools.lib.test_script import add_provision_check_override_param, assert_provisioning_status_ok from tools.lib.test_server import test_server_running usage = """test-api [options]""" parser = argparse.ArgumentParser(usage) -parser.add_argument( - "--force", action="store_true", help="Run tests despite possible provisioning problems." -) +add_provision_check_override_param(parser) options = parser.parse_args() assert_provisioning_status_ok(options.force) diff --git a/tools/test-backend b/tools/test-backend index face30427b..22676ac3be 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -192,7 +192,10 @@ def main() -> None: os.environ.pop("http_proxy", "") os.environ.pop("https_proxy", "") - from tools.lib.test_script import assert_provisioning_status_ok + from tools.lib.test_script import ( + add_provision_check_override_param, + assert_provisioning_status_ok, + ) from zerver.lib.test_fixtures import ( remove_test_run_directories, update_test_databases_if_required, @@ -239,7 +242,7 @@ def main() -> None: "tests in. Default is the number of logical CPUs", ) parser.add_argument("--profile", action="store_true", help="Profile test runtime.") - parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") + add_provision_check_override_param(parser) parser.add_argument( "--no-shallow", action="store_true", diff --git a/tools/test-help-documentation b/tools/test-help-documentation index b6ad9ca477..5764186991 100755 --- a/tools/test-help-documentation +++ b/tools/test-help-documentation @@ -13,8 +13,13 @@ from lib import sanity_check sanity_check.check_venv(__file__) +os.chdir(ZULIP_PATH) +sys.path.insert(0, ZULIP_PATH) + +from tools.lib.test_script import add_provision_check_override_param + parser = argparse.ArgumentParser() -parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") +add_provision_check_override_param(parser) parser.add_argument( "--skip-external-links", dest="skip_external_link_check", @@ -23,8 +28,6 @@ parser.add_argument( ) options = parser.parse_args() -os.chdir(ZULIP_PATH) -sys.path.insert(0, ZULIP_PATH) from tools.lib.test_server import test_server_running os.makedirs("var/help-documentation", exist_ok=True) diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 17b52a8b6a..cd6768ea32 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -167,15 +167,15 @@ EXEMPT_FILES = { "static/js/zulip_test.js", } +from tools.lib.test_script import add_provision_check_override_param, assert_provisioning_status_ok + parser = argparse.ArgumentParser(USAGE) parser.add_argument("--coverage", action="store_true", help="Get coverage report") -parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") +add_provision_check_override_param(parser) parser.add_argument("args", nargs=argparse.REMAINDER) options = parser.parse_args() individual_files = options.args -from tools.lib.test_script import assert_provisioning_status_ok - assert_provisioning_status_ok(options.force) diff --git a/tools/test-js-with-puppeteer b/tools/test-js-with-puppeteer index 6af72b932c..c3b6c19b10 100755 --- a/tools/test-js-with-puppeteer +++ b/tools/test-js-with-puppeteer @@ -25,15 +25,6 @@ usage = """test-js-with-puppeteer [options] test-js-with-puppeteer 09 # Run a single test file 09-navigation.js test-js-with-puppeteer 01-login.js 03-narrow.js # Run a few test files test-js-with-puppeteer 01 03 # Run a few test files, 01-login.js and 03-narrow.js here""" -parser = argparse.ArgumentParser(usage) - -parser.add_argument("--interactive", action="store_true", help="Run tests interactively") -parser.add_argument("--force", action="store_true", help="Run tests despite possible problems.") -parser.add_argument( - "tests", nargs=argparse.REMAINDER, help="Specific tests to run; by default, runs all tests" -) - -options = parser.parse_args() sys.path.insert(0, ZULIP_PATH) @@ -45,12 +36,23 @@ sanity_check.check_venv(__file__) from typing import Iterable, Tuple from tools.lib.test_script import ( + add_provision_check_override_param, assert_provisioning_status_ok, find_js_test_files, prepare_puppeteer_run, ) from tools.lib.test_server import test_server_running +parser = argparse.ArgumentParser(usage) + +parser.add_argument("--interactive", action="store_true", help="Run tests interactively") +add_provision_check_override_param(parser) +parser.add_argument( + "tests", nargs=argparse.REMAINDER, help="Specific tests to run; by default, runs all tests" +) + +options = parser.parse_args() + def run_tests(files: Iterable[str], external_host: str) -> None: test_dir = os.path.join(ZULIP_PATH, "frontend_tests/puppeteer_tests")