Skip to content

Commit

Permalink
test: remove some unused paver references from CI scripts (#34468)
Browse files Browse the repository at this point in the history
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] #34467
  • Loading branch information
kdmccormick authored Apr 4, 2024
1 parent fe13884 commit 5a78548
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 78 deletions.
1 change: 0 additions & 1 deletion .github/workflows/quality-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
6 changes: 1 addition & 5 deletions pavelib/prereqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
7 changes: 0 additions & 7 deletions pavelib/utils/envs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
80 changes: 16 additions & 64 deletions scripts/generic-ci-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand Down Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ passenv =
SELENIUM_BROWSER
SELENIUM_HOST
SELENIUM_PORT
SHARD
SKIP_NPM_INSTALL
SSH_AUTH_SOCK
STUDIO_CFG
Expand Down

0 comments on commit 5a78548

Please sign in to comment.