From af4b79b872109e065fb3ac94816c8a34bbf086dc Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Wed, 3 Apr 2024 13:14:27 -0400 Subject: [PATCH] test: remove some unused paver references from CI scripts All CI used to go through scripts/generic-ci-tests.sh, which is a wrapper around various `paver` test/linting/check invocations. These days, most edx-platform CI checks just invoke their tools (pylint, pycodestyle, pytest, etc.) directly. In anticipation of the proposed Paver deprecation [1], let's remove the parts of this script that aren't used any more, including several `paver` command invocations. This should have no impact on CI. Furthermore, we are able to remove the SHARD environment variable, which was formely used to split unit and quality checks up into smaller pieces. Unit tests and pylint checks now have their own separate sharding logic, so there is only one "quality" shard remaining (SHARD=4, ie generic quality checks), thus we don't need a SHARD variable at all. [1] https://github.com/openedx/edx-platform/issues/34467 --- .github/workflows/quality-checks.yml | 1 - pavelib/prereqs.py | 6 +-- pavelib/utils/envs.py | 7 --- scripts/generic-ci-tests.sh | 80 ++++++---------------------- tox.ini | 1 - 5 files changed, 17 insertions(+), 78 deletions(-) diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index a5e65fb67e94..97b6d08a7974 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -65,7 +65,6 @@ jobs: env: TEST_SUITE: quality SCRIPT_TO_RUN: ./scripts/generic-ci-tests.sh - SHARD: 4 PIP_SRC: ${{ runner.temp }} TARGET_BRANCH: ${{ github.base_ref }} run: | diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index 8d736edb8f9d..c234c520593a 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -148,11 +148,7 @@ def node_prereqs_installation(): # NPM installs hang sporadically. Log the installation process so that we # determine if any packages are chronic offenders. - shard_str = os.getenv('SHARD', None) - if shard_str: - npm_log_file_path = f'{Env.GEN_LOG_DIR}/npm-install.{shard_str}.log' - else: - npm_log_file_path = f'{Env.GEN_LOG_DIR}/npm-install.log' + npm_log_file_path = f'{Env.GEN_LOG_DIR}/npm-install.log' npm_log_file = open(npm_log_file_path, 'wb') # lint-amnesty, pylint: disable=consider-using-with npm_command = 'npm ci --verbose'.split() diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index ed2553a42fa2..ae99d8d6d3f1 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -64,13 +64,6 @@ class Env: # Which Python version should be used in xdist workers? PYTHON_VERSION = os.environ.get("PYTHON_VERSION", "2.7") - # If set, put reports for run in "unique" directories. - # The main purpose of this is to ensure that the reports can be 'slurped' - # in the main jenkins flow job without overwriting the reports from other - # build steps. For local development/testing, this shouldn't be needed. - if os.environ.get("SHARD", None): - shard_str = "shard_{}".format(os.environ.get("SHARD")) - # Directory that videos are served from VIDEO_SOURCE_DIR = REPO_ROOT / "test_root" / "data" / "video" diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh index f854a3e36cda..4ba41d4bfe4f 100755 --- a/scripts/generic-ci-tests.sh +++ b/scripts/generic-ci-tests.sh @@ -5,7 +5,8 @@ set -e # # generic-ci-tests.sh # -# Execute all tests for edx-platform. +# Execute some tests for edx-platform. +# (Most other tests are run by invoking `pytest`, `pylint`, etc. directly) # # This script can be called from CI jobs that define # these environment variables: @@ -14,41 +15,12 @@ set -e # Possible values are: # # - "quality": Run the quality (pycodestyle/pylint) checks -# - "lms-unit": Run the LMS Python unit tests -# - "cms-unit": Run the CMS Python unit tests # - "js-unit": Run the JavaScript tests -# - "pavelib-unit": Run Python unit tests from the pavelib/lib directory # - "pavelib-js-unit": Run the JavaScript tests and the Python unit # tests from the pavelib/lib directory # -# `SHARD` is a number indicating which subset of the tests to build. -# -# For "lms-unit", the tests are put into shard groups -# using the 'attr' decorator (e.g. "@attr(shard=1)"). Anything with -# the 'shard=n' attribute will run in the nth shard. If there isn't a -# shard explicitly assigned, the test will run in the last shard. -# -# Jenkins-specific configuration details: -# -# - The edx-platform git repository is checked out by the Jenkins git plugin. -# - Jenkins logs in as user "jenkins" -# - The Jenkins file system root is "/home/jenkins" -# - An init script creates a virtualenv at "/home/jenkins/edx-venv" -# with some requirements pre-installed (such as scipy) -# -# Jenkins worker setup: -# See the edx/configuration repo for Jenkins worker provisioning scripts. -# The provisioning scripts install requirements that this script depends on! -# ############################################################################### -# If the environment variable 'SHARD' is not set, default to 'all'. -# This could happen if you are trying to use this script from -# jenkins and do not define 'SHARD' in your multi-config project. -# Note that you will still need to pass a value for 'TEST_SUITE' -# or else no tests will be executed. -SHARD=${SHARD:="all"} - # Clean up previous builds git clean -qxfd @@ -105,40 +77,20 @@ case "$TEST_SUITE" in mkdir -p reports - case "$SHARD" in - 1) - echo "Finding pylint violations and storing in report..." - run_paver_quality run_pylint --system=common || { EXIT=1; } - ;; - - 2) - echo "Finding pylint violations and storing in report..." - run_paver_quality run_pylint --system=lms || { EXIT=1; } - ;; - - 3) - echo "Finding pylint violations and storing in report..." - run_paver_quality run_pylint --system="cms,openedx,pavelib" || { EXIT=1; } - ;; - - 4) - echo "Finding fixme's and storing report..." - run_paver_quality find_fixme || { EXIT=1; } - echo "Finding pycodestyle violations and storing report..." - run_paver_quality run_pep8 || { EXIT=1; } - echo "Finding ESLint violations and storing report..." - run_paver_quality run_eslint -l "$ESLINT_THRESHOLD" || { EXIT=1; } - echo "Finding Stylelint violations and storing report..." - run_paver_quality run_stylelint || { EXIT=1; } - echo "Running xss linter report." - run_paver_quality run_xsslint -t "$XSSLINT_THRESHOLDS" || { EXIT=1; } - echo "Running PII checker on all Django models..." - run_paver_quality run_pii_check || { EXIT=1; } - echo "Running reserved keyword checker on all Django models..." - run_paver_quality check_keywords || { EXIT=1; } - ;; - - esac + echo "Finding fixme's and storing report..." + run_paver_quality find_fixme || { EXIT=1; } + echo "Finding pycodestyle violations and storing report..." + run_paver_quality run_pep8 || { EXIT=1; } + echo "Finding ESLint violations and storing report..." + run_paver_quality run_eslint -l "$ESLINT_THRESHOLD" || { EXIT=1; } + echo "Finding Stylelint violations and storing report..." + run_paver_quality run_stylelint || { EXIT=1; } + echo "Running xss linter report." + run_paver_quality run_xsslint -t "$XSSLINT_THRESHOLDS" || { EXIT=1; } + echo "Running PII checker on all Django models..." + run_paver_quality run_pii_check || { EXIT=1; } + echo "Running reserved keyword checker on all Django models..." + run_paver_quality check_keywords || { EXIT=1; } # Need to create an empty test result so the post-build # action doesn't fail the build. diff --git a/tox.ini b/tox.ini index 3069c04ca4c7..9b1937f3640a 100644 --- a/tox.ini +++ b/tox.ini @@ -46,7 +46,6 @@ passenv = SELENIUM_BROWSER SELENIUM_HOST SELENIUM_PORT - SHARD SKIP_NPM_INSTALL SSH_AUTH_SOCK STUDIO_CFG