From 897cb3617fc3eddcecd82299ce2444fd638bb4df Mon Sep 17 00:00:00 2001 From: UsamaSadiq Date: Thu, 25 Aug 2022 16:31:47 +0500 Subject: [PATCH] refactor!: delete common/lib and related usages --- .coveragerc-local | 1 - .github/CODEOWNERS | 3 -- .github/actions/verify-tests-count/action.yml | 2 +- .github/workflows/ci-static-analysis.yml | 2 +- .github/workflows/docs-build-check.yml | 2 +- .github/workflows/js-tests.yml | 4 +- .github/workflows/migrations-check-mysql8.yml | 2 +- .github/workflows/migrations-check.yml | 2 +- .github/workflows/pylint-checks.yml | 2 +- .github/workflows/quality-checks.yml | 6 ++- .github/workflows/static-assets-check.yml | 2 +- .github/workflows/unit-tests-gh-hosted.yml | 4 +- .github/workflows/unit-tests.yml | 2 +- .../workflows/verify-gha-unit-tests-count.yml | 2 +- Dockerfile | 14 ++---- Makefile | 18 +++++-- common/lib/conftest.py | 29 ----------- common/lib/pytest.ini | 22 --------- .../0013-library-reference-content-block.rst | 2 +- docs/guides/docstrings/common_lib.rst | 9 ---- pavelib/prereqs.py | 2 + pavelib/tests.py | 14 +++--- pavelib/utils/envs.py | 10 +--- pavelib/utils/test/suites/pytest_suite.py | 8 +--- pavelib/utils/test/suites/python_suite.py | 2 +- pavement.py | 2 - pylintrc | 41 ++++++++-------- requirements/edx/base.in | 1 - requirements/edx/base.txt | 2 - requirements/edx/development.txt | 2 - requirements/edx/local.in | 2 - requirements/edx/testing.txt | 2 - scripts/ci-runner.Dockerfile | 1 - scripts/circle-ci-tests.sh | 4 +- scripts/generic-ci-tests.sh | 10 ++-- scripts/post-pip-compile.sh | 48 ------------------- scripts/unit-tests.sh | 20 ++++---- scripts/verify-dunder-init.sh | 9 ---- scripts/xdist/get_worker_test_list.py | 5 +- xmodule/modulestore/tests/utils.py | 5 +- 40 files changed, 92 insertions(+), 228 deletions(-) delete mode 100644 common/lib/conftest.py delete mode 100644 common/lib/pytest.ini delete mode 100644 docs/guides/docstrings/common_lib.rst delete mode 100644 requirements/edx/local.in delete mode 100755 scripts/post-pip-compile.sh diff --git a/.coveragerc-local b/.coveragerc-local index f14fdeea9ce2..807e68b11113 100644 --- a/.coveragerc-local +++ b/.coveragerc-local @@ -4,7 +4,6 @@ data_file = reports/.coverage source = cms common/djangoapps - common/lib/xmodule lms openedx pavelib diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 1ee6f5df406a..b93c1752f7bb 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -17,9 +17,6 @@ openedx/features/course_experience/ xmodule/ # Core Extensions -common/lib/xmodule/xmodule/capa_module.py -common/lib/xmodule/xmodule/html_module.py -common/lib/xmodule/xmodule/video_module lms/djangoapps/discussion/ lms/djangoapps/edxnotes diff --git a/.github/actions/verify-tests-count/action.yml b/.github/actions/verify-tests-count/action.yml index d7229af5050d..8260827436a9 100644 --- a/.github/actions/verify-tests-count/action.yml +++ b/.github/actions/verify-tests-count/action.yml @@ -7,7 +7,7 @@ runs: shell: bash run: | echo "root_cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test cms/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - echo "root_lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test lms/ openedx/ common/djangoapps/ common/lib/ xmodule/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "root_lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test lms/ openedx/ common/djangoapps/ xmodule/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - name: get GHA unit test paths shell: bash diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index 0a83b142a74b..94b6b30600a8 100644 --- a/.github/workflows/ci-static-analysis.yml +++ b/.github/workflows/ci-static-analysis.yml @@ -37,7 +37,7 @@ jobs: restore-keys: ${{ runner.os }}-pip- - name: Install python dependencies - run: pip install -r requirements/edx/development.txt + run: make dev-requirements - name: Static code analysis run: make check-types diff --git a/.github/workflows/docs-build-check.yml b/.github/workflows/docs-build-check.yml index 4cb573167781..10dd000ce0e2 100644 --- a/.github/workflows/docs-build-check.yml +++ b/.github/workflows/docs-build-check.yml @@ -41,7 +41,7 @@ jobs: restore-keys: ${{ runner.os }}-pip- - name: Install python dependencies - run: pip install -r requirements/edx/development.txt + run: make dev-requirements - name: Install docs requirements run: pip install -r requirements/edx/doc.txt diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index dc5ebaa81bb3..4975253e28c5 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -28,7 +28,7 @@ jobs: uses: actions/setup-node@v2 with: node-version: ${{ matrix.node-version }} - + - name: Setup npm run: npm i -g npm@8.5.x @@ -64,7 +64,7 @@ jobs: - name: Install Required Python Dependencies run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/base.txt + make base-requirements - uses: c-hive/gha-npm-cache@v1 - name: Run JS Tests diff --git a/.github/workflows/migrations-check-mysql8.yml b/.github/workflows/migrations-check-mysql8.yml index 9ebdf5f978dc..6c817abb0efd 100644 --- a/.github/workflows/migrations-check-mysql8.yml +++ b/.github/workflows/migrations-check-mysql8.yml @@ -51,7 +51,7 @@ jobs: - name: Install Python dependencies run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/development.txt + make dev-requirements pip uninstall -y mysqlclient pip install --no-binary mysqlclient mysqlclient pip uninstall -y xmlsec diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 6e20ac2dd412..7fe2acad86dd 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -61,7 +61,7 @@ jobs: - name: Install Python dependencies run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/development.txt + make dev-requirements pip uninstall -y mysqlclient pip install --no-binary mysqlclient mysqlclient pip uninstall -y xmlsec diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index 8c57e9ee81e1..dc39ed433a78 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -59,7 +59,7 @@ jobs: - name: Install Required Python Dependencies run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/development.txt --src ${{ runner.temp }} + make dev-requirements pip uninstall -y mysqlclient pip install --no-binary mysqlclient mysqlclient diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index dfb19a09278e..e28be7aba70e 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -16,7 +16,7 @@ jobs: os: [ ubuntu-20.04 ] python-version: [ 3.8 ] node-version: [ 16 ] - + steps: - uses: actions/checkout@v2 @@ -60,7 +60,9 @@ jobs: PIP_SRC_DIR: ${{ runner.temp }} run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/testing.txt -r requirements/edx/django.txt --src $PIP_SRC_DIR + pip install -r requirements/edx/testing.txt --src $PIP_SRC_DIR + pip install -e . --src $PIP_SRC_DIR + pip install -r requirements/edx/django.txt - name: Run Quality Tests env: diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index d1648efd2cc8..4da597d69b1e 100644 --- a/.github/workflows/static-assets-check.yml +++ b/.github/workflows/static-assets-check.yml @@ -50,7 +50,7 @@ jobs: - name: Install Required Python Dependencies run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/base.txt + make base-requirements - name: Initiate Mongo DB Service run: sudo systemctl start mongod diff --git a/.github/workflows/unit-tests-gh-hosted.yml b/.github/workflows/unit-tests-gh-hosted.yml index 9fffd785544e..c493bcdbf374 100644 --- a/.github/workflows/unit-tests-gh-hosted.yml +++ b/.github/workflows/unit-tests-gh-hosted.yml @@ -67,6 +67,8 @@ jobs: run: | pip install -r requirements/pip.txt pip install -r requirements/edx/development.txt --src ${{ runner.temp }} +# edx-platform installs some Python projects from within the edx-platform repo itself. + pip install -e . pip install "django~=${{ matrix.django-version }}.0" - name: Setup and run tests @@ -106,7 +108,7 @@ jobs: - name: Install Required Python Dependencies run: | pip install -r requirements/pip.txt - pip install -r requirements/edx/development.txt --src ${{ runner.temp }} + make dev-requirements pip install "django~=${{ matrix.django-version }}.0" - name: verify unit tests count diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 3b17418e58e0..7b970cde80d8 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -61,7 +61,7 @@ jobs: - name: install requirements run: | sudo pip install -r requirements/pip.txt - sudo pip install --exists-action='w' -r requirements/edx/testing.txt + sudo make test-requirements if [[ "${{ matrix.django-version }}" == "pinned" ]]; then sudo pip install -r requirements/edx/django.txt else diff --git a/.github/workflows/verify-gha-unit-tests-count.yml b/.github/workflows/verify-gha-unit-tests-count.yml index 8b5701af4830..e1ae9696783b 100644 --- a/.github/workflows/verify-gha-unit-tests-count.yml +++ b/.github/workflows/verify-gha-unit-tests-count.yml @@ -18,7 +18,7 @@ jobs: - name: install requirements run: | sudo pip install -r requirements/pip.txt - sudo pip install --exists-action='w' -r requirements/edx/testing.txt + sudo make test-requirements - name: verify unit tests count uses: ./.github/actions/verify-tests-count diff --git a/Dockerfile b/Dockerfile index 7618fe524b66..9b8ead8570ea 100644 --- a/Dockerfile +++ b/Dockerfile @@ -91,15 +91,9 @@ RUN python3.8 -m venv "$VIRTUAL_ENV" # Install Python requirements. # Requires copying over requirements files, but not entire repository. -# We filter out the local ('common/*' and 'openedx/*', and '.') Python projects, -# because those require code in order to be installed. They will be installed -# later. This step can be simplified when the local projects are dissolved -# (see https://openedx.atlassian.net/browse/BOM-2579). COPY requirements requirements -RUN sed '/^-e \(common\/\|openedx\/\|.\)/d' requirements/edx/base.txt \ - > requirements/edx/base-minus-local.txt RUN pip install -r requirements/pip.txt -RUN pip install -r requirements/edx/base-minus-local.txt +RUN pip install -r requirements/edx/base.txt # Set up a Node environment and install Node requirements. # Must be done after Python requirements, since nodeenv is installed @@ -114,10 +108,8 @@ RUN npm set progress=false && npm install # Copy over remaining parts of repository (including all code). COPY . . -# Install Python requirements again in order to capture local projects, which -# were skipped earlier. This should be much quicker than if were installing -# all requirements from scratch. -RUN pip install -r requirements/edx/base.txt +# Install Python requirements again in order to capture local projects +RUN pip install -e . ################################################## # Define LMS docker-based non-dev target. diff --git a/Makefile b/Makefile index 0c1076fa9b83..0bc1766a402b 100644 --- a/Makefile +++ b/Makefile @@ -66,10 +66,24 @@ pull: ## update the Docker image used by "make shell" pre-requirements: ## install Python requirements for running pip-tools pip install -qr requirements/edx/pip-tools.txt +local-requirements: +# edx-platform installs some Python projects from within the edx-platform repo itself. + pip install -e . + +dev-requirements: local-requirements + pip install -qr requirements/edx/development.txt + +base-requirements: local-requirements + pip install -qr requirements/edx/base.txt + +test-requirements: local-requirements + pip install --exists-action='w' -qr requirements/edx/testing.txt + requirements: pre-requirements ## install development environment requirements @# The "$(wildcard..)" is to include private.txt if it exists, and make no mention @# of it if it does not. Shell wildcarding can't do that with default options. pip-sync -q requirements/edx/development.txt $(wildcard requirements/edx/private.txt) + pip install -e . shell: ## launch a bash shell in a Docker container with all edx-platform dependencies installed docker run -it -e "NO_PYTHON_UNINSTALL=1" -e "PIP_INDEX_URL=https://pypi.python.org/simple" -e TERM \ @@ -116,9 +130,7 @@ compile-requirements: $(COMMON_CONSTRAINTS_TXT) ## Re-compile *.in requirements pip-compile -v --no-emit-trusted-host --no-emit-index-url $$REBUILD ${COMPILE_OPTS} -o $$f.txt $$f.in || exit 1; \ export REBUILD=''; \ done - # Post process all of the files generated above to work around open pip-tools issues - scripts/post-pip-compile.sh $(REQ_FILES:=.txt) - # Let tox control the Django version for tests + # Let tox control the Django version for tests grep -e "^django==" requirements/edx/base.txt > requirements/edx/django.txt sed '/^[dD]jango==/d' requirements/edx/testing.txt > requirements/edx/testing.tmp mv requirements/edx/testing.tmp requirements/edx/testing.txt diff --git a/common/lib/conftest.py b/common/lib/conftest.py deleted file mode 100644 index 8b5b151a255a..000000000000 --- a/common/lib/conftest.py +++ /dev/null @@ -1,29 +0,0 @@ -"""Code run by pylint before running any tests.""" - -# Patch the xml libs before anything else. - - -import pytest - -from openedx.core.lib.safe_lxml import defuse_xml_libs - -# This import is needed for pytest plugin configuration, so please avoid deleting this during refactoring -from openedx.core.pytest_hooks import pytest_configure # pylint: disable=unused-import - -defuse_xml_libs() - - -@pytest.fixture(autouse=True) -def no_webpack_loader(monkeypatch): # lint-amnesty, pylint: disable=missing-function-docstring - monkeypatch.setattr( - "webpack_loader.templatetags.webpack_loader.render_bundle", - lambda entry, extension=None, config='DEFAULT', attrs='': '' - ) - monkeypatch.setattr( - "webpack_loader.utils.get_as_tags", - lambda entry, extension=None, config='DEFAULT', attrs='': [] - ) - monkeypatch.setattr( - "webpack_loader.utils.get_files", - lambda entry, extension=None, config='DEFAULT', attrs='': [] - ) diff --git a/common/lib/pytest.ini b/common/lib/pytest.ini deleted file mode 100644 index aed75b7b1c48..000000000000 --- a/common/lib/pytest.ini +++ /dev/null @@ -1,22 +0,0 @@ -[pytest] -# Note: The first file of settings found is used, there is no combining, so -# this file is used for the tests in the common/lib tree, and setup.cfg is ignored. -# Details at https://docs.pytest.org/en/latest/reference/customize.html - -# Use the LMS settings for these tests; should work with CMS just as well though: -DJANGO_SETTINGS_MODULE = lms.envs.test -addopts = --nomigrations --reuse-db --durations=20 --json-report --json-report-omit keywords streams collectors log traceback tests --json-report-file=none -# Enable default handling for all warnings, including those that are ignored by default; -# but hide rate-limit warnings (because we deliberately don't throttle test user logins) -# and field_data deprecation warnings (because fixing them requires a major low-priority refactoring) -filterwarnings = - default - ignore:No request passed to the backend, unable to rate-limit:UserWarning - ignore::xblock.exceptions.FieldDataDeprecationWarning - ignore::django.utils.deprecation.RemovedInDjango40Warning - ignore::django.utils.deprecation.RemovedInDjango41Warning -markers = - mongo -norecursedirs = .cache -python_classes = -python_files = tests.py test_*.py tests_*.py *_tests.py __init__.py diff --git a/docs/decisions/0013-library-reference-content-block.rst b/docs/decisions/0013-library-reference-content-block.rst index f5d5edfcfdb2..cd842d8b0f79 100644 --- a/docs/decisions/0013-library-reference-content-block.rst +++ b/docs/decisions/0013-library-reference-content-block.rst @@ -61,7 +61,7 @@ out of the set selected by the author. Current Architecture/Implementation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Currently, courses are stored in Modulestore and libraries can either be stored in -Modulestore or Blockstore. The [library_sourced_block](https://github.com/openedx/edx-platform/blob/master/common/lib/xmodule/xmodule/library_sourced_block.py) +Modulestore or Blockstore. The [library_sourced_block](https://github.com/openedx/edx-platform/blob/master/xmodule/library_sourced_block.py) is used to make a copy of blockstore-based v2 content library block and store it in Modulestore itself as the child. diff --git a/docs/guides/docstrings/common_lib.rst b/docs/guides/docstrings/common_lib.rst deleted file mode 100644 index 725f712db915..000000000000 --- a/docs/guides/docstrings/common_lib.rst +++ /dev/null @@ -1,9 +0,0 @@ -common/lib -********** - -This directory contains libraries which are installed locally so they can be -imported by name with no package hierarchy. They will most likely be split -out from edx-platform into separate packages at some point. - -.. toctree:: - :maxdepth: 2 diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index 89a53a885dae..3b588f65faf6 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -164,6 +164,8 @@ def python_prereqs_installation(): """ Installs Python prerequisites """ + # edx-platform installs some Python projects from within the edx-platform repo itself. + sh(f"pip install -e .") for req_file in PYTHON_REQ_FILES: pip_install_req_file(req_file) diff --git a/pavelib/tests.py b/pavelib/tests.py index b1c4ff7a4a14..28988e8f05e9 100644 --- a/pavelib/tests.py +++ b/pavelib/tests.py @@ -34,7 +34,7 @@ ("fasttest", "a", "Run without collectstatic"), make_option( "--django_version", dest="django_version", - help="Run against which Django version (1.8, 1.9, 1.10, -or- 1.11)." + help="Run against which Django version (3.2)." ), make_option( "--eval-attr", dest="eval_attr", @@ -109,7 +109,7 @@ def test_system(options, passthrough_options): django_version = getattr(options, 'django_version', None) assert system in (None, 'lms', 'cms') - assert django_version in (None, '1.8', '1.9', '1.10', '1.11') + assert django_version in (None, '3.2') if hasattr(options.test_system, 'with_wtw'): call_task('fetch_coverage_test_selection_data', options={ @@ -163,7 +163,7 @@ def test_system(options, passthrough_options): ("fail-fast", "x", "Run only failed tests"), make_option( "--django_version", dest="django_version", - help="Run against which Django version (1.8, 1.9, 1.10, -or- 1.11)." + help="Run against which Django version (3.2)." ), make_option( "--eval-attr", dest="eval_attr", @@ -197,20 +197,20 @@ def test_system(options, passthrough_options): @timed def test_lib(options, passthrough_options): """ - Run tests for common/lib/ and pavelib/ (paver-tests) + Run tests for pavelib/ (paver-tests) """ lib = getattr(options, 'lib', None) test_id = getattr(options, 'test_id', lib) django_version = getattr(options, 'django_version', None) - assert django_version in (None, '1.8', '1.9', '1.10', '1.11') + assert django_version in (None, '3.2') if test_id: # Testing a single test id. if '/' in test_id: lib = '/'.join(test_id.split('/')[0:3]) else: - lib = 'common/lib/' + test_id.split('.')[0] + lib = 'pavelib/paver_tests' + test_id.split('.')[0] options.test_lib['test_id'] = test_id lib_tests = [suites.LibTestSuite( lib, @@ -218,7 +218,7 @@ def test_lib(options, passthrough_options): **options.test_lib )] else: - # Testing all common/lib test dirs - plus pavelib. + # Testing all tests within pavelib/paver_tests dir. lib_tests = [ suites.LibTestSuite( d, diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index 6881b0ea1340..ef8ce87e5662 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -209,15 +209,9 @@ class Env: JS_REPORT_DIR = REPORT_DIR / 'javascript' - # Directories used for common/lib/tests + # Directories used for pavelib/ tests IGNORED_TEST_DIRS = ('__pycache__', '.cache', '.pytest_cache') - LIB_TEST_DIRS = [] - for item in (REPO_ROOT / "common/lib").listdir(): - dir_name = (REPO_ROOT / 'common/lib' / item) - if dir_name.isdir() and not dir_name.endswith(IGNORED_TEST_DIRS): - LIB_TEST_DIRS.append(path("common/lib") / item.basename()) - LIB_TEST_DIRS.append(path("pavelib/paver_tests")) - LIB_TEST_DIRS.append(path("scripts/xsslint/tests")) + LIB_TEST_DIRS = [path("pavelib/paver_tests"), path("scripts/xsslint/tests")] # Directory for i18n test reports I18N_REPORT_DIR = REPORT_DIR / 'i18n' diff --git a/pavelib/utils/test/suites/pytest_suite.py b/pavelib/utils/test/suites/pytest_suite.py index 40744ce5130f..06de92334d34 100644 --- a/pavelib/utils/test/suites/pytest_suite.py +++ b/pavelib/utils/test/suites/pytest_suite.py @@ -260,7 +260,7 @@ def included(path): class LibTestSuite(PytestSuite): """ - TestSuite for edx-platform/common/lib python unit tests + TestSuite for edx-platform/pavelib/paver_tests python unit tests """ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -321,11 +321,7 @@ def cmd(self): for rsync_dir in Env.rsync_dirs(): cmd.append(f'--rsyncdir {rsync_dir}') # "--rsyncdir" throws off the configuration root, set it explicitly - if 'common/lib' in self.test_id: - cmd.append('--rootdir=common/lib') - cmd.append('-c common/lib/pytest.ini') - elif 'pavelib/paver_tests' in self.test_id: - cmd.append('--rootdir=pavelib/paver_tests') + cmd.append('--rootdir=pavelib/paver_tests') else: if self.processes == -1: cmd.append('-n auto') diff --git a/pavelib/utils/test/suites/python_suite.py b/pavelib/utils/test/suites/python_suite.py index f1944251a340..f7b4f3450daa 100644 --- a/pavelib/utils/test/suites/python_suite.py +++ b/pavelib/utils/test/suites/python_suite.py @@ -37,7 +37,7 @@ def __enter__(self): def _default_subsuites(self): """ The default subsuites to be run. They include lms, cms, - and all of the libraries in common/lib. + and all of the libraries in pavelib/paver_tests. """ lib_suites = [ LibTestSuite(d, **self.opts) for d in Env.LIB_TEST_DIRS diff --git a/pavement.py b/pavement.py index a9afd673b923..f07bd51ece8a 100644 --- a/pavement.py +++ b/pavement.py @@ -5,8 +5,6 @@ # takes precedence over anything else installed in the virtualenv. # In local dev, we usually don't need to do this, because Python # automatically puts the current working directory on the system path. -# In Jenkins, however, we have multiple copies of the edx-platform repo, -# each of which run "pip install -e ." (as part of requirements/edx/local.in) # Until we re-run pip install, the other copies of edx-platform could # take precedence, leading to some very strange results. sys.path.insert(0, os.path.dirname(__file__)) diff --git a/pylintrc b/pylintrc index 1ab6ca4c6d63..14b4cc0afbc1 100644 --- a/pylintrc +++ b/pylintrc @@ -72,10 +72,10 @@ persistent = yes load-plugins = edx_lint.pylint,pylint_django_settings,pylint_django,pylint_celery,pylint_pytest [MESSAGES CONTROL] -enable = +enable = blacklisted-name, line-too-long, - + abstract-class-instantiated, abstract-method, access-member-before-definition, @@ -184,26 +184,26 @@ enable = used-before-assignment, using-constant-test, yield-outside-function, - + astroid-error, fatal, method-check-failed, parse-error, raw-checker-failed, - + empty-docstring, invalid-characters-in-docstring, missing-docstring, wrong-spelling-in-comment, wrong-spelling-in-docstring, - + unused-argument, unused-import, unused-variable, - + eval-used, exec-used, - + bad-classmethod-argument, bad-mcs-classmethod-argument, bad-mcs-method-argument, @@ -234,30 +234,30 @@ enable = unneeded-not, useless-else-on-loop, wrong-assert-type, - + deprecated-method, deprecated-module, - + too-many-boolean-expressions, too-many-nested-blocks, too-many-statements, - + wildcard-import, wrong-import-order, wrong-import-position, - + missing-final-newline, mixed-line-endings, trailing-newlines, trailing-whitespace, unexpected-line-ending-format, - + bad-inline-option, bad-option-value, deprecated-pragma, unrecognized-inline-option, useless-suppression, -disable = +disable = bad-indentation, consider-using-f-string, duplicate-code, @@ -281,10 +281,10 @@ disable = unspecified-encoding, unused-wildcard-import, use-maxsplit-arg, - + feature-toggle-needs-doc, illegal-waffle-usage, - + logging-fstring-interpolation, native-string, import-outside-toplevel, @@ -308,6 +308,7 @@ disable = unused-argument, unsubscriptable-object, abstract-method, + wrong-import-order, no-self-use, [REPORTS] @@ -351,7 +352,7 @@ ignore-imports = no ignore-mixin-members = yes ignored-classes = SQLObject unsafe-load-any-extension = yes -generated-members = +generated-members = REQUEST, acl_users, aq_parent, @@ -377,7 +378,7 @@ generated-members = [VARIABLES] init-import = no dummy-variables-rgx = _|dummy|unused|.*_unused -additional-builtins = +additional-builtins = [CLASSES] defining-attr-methods = __init__,__new__,setUp @@ -398,9 +399,9 @@ max-public-methods = 20 [IMPORTS] deprecated-modules = regsub,TERMIOS,Bastion,rexec -import-graph = -ext-import-graph = -int-import-graph = +import-graph = +ext-import-graph = +int-import-graph = [EXCEPTIONS] overgeneral-exceptions = Exception diff --git a/requirements/edx/base.in b/requirements/edx/base.in index b4c854dc9bd0..9b87a3c4e870 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -11,7 +11,6 @@ -c ../constraints.txt -r github.in # Forks and other dependencies not yet on PyPI --r local.in # Packages in edx-platform which have their own setup.py -r paver.txt # Requirements for running paver commands # Please follow these guidelines whenever you change this file: diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f8f8cf6e9aaa..fdc704ca4965 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -12,8 +12,6 @@ # via -r requirements/edx/github.in -e git+https://github.com/openedx/olxcleaner.git@2f0d6c7f126cbd69c9724b7b57a0b2565330a297#egg=olxcleaner # via -r requirements/edx/github.in --e . - # via -r requirements/edx/local.in -e git+https://github.com/openedx/RateXBlock.git@2.0.1#egg=rate-xblock # via -r requirements/edx/github.in -e git+https://github.com/openedx/xblock-google-drive.git@2d176468e33c0713c911b563f8f65f7cf232f5b6#egg=xblock-google-drive diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 491f428c88cf..52a8d7fd7acd 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -12,8 +12,6 @@ # via -r requirements/edx/testing.txt -e git+https://github.com/openedx/olxcleaner.git@2f0d6c7f126cbd69c9724b7b57a0b2565330a297#egg=olxcleaner # via -r requirements/edx/testing.txt --e . - # via -r requirements/edx/testing.txt -e git+https://github.com/openedx/RateXBlock.git@2.0.1#egg=rate-xblock # via -r requirements/edx/testing.txt -e git+https://github.com/openedx/xblock-google-drive.git@2d176468e33c0713c911b563f8f65f7cf232f5b6#egg=xblock-google-drive diff --git a/requirements/edx/local.in b/requirements/edx/local.in deleted file mode 100644 index 776e718daa80..000000000000 --- a/requirements/edx/local.in +++ /dev/null @@ -1,2 +0,0 @@ -# Python libraries to install that are local to the edx-platform repo --e . diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 393bd611452e..66512eabe86a 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -12,8 +12,6 @@ # via -r requirements/edx/base.txt -e git+https://github.com/openedx/olxcleaner.git@2f0d6c7f126cbd69c9724b7b57a0b2565330a297#egg=olxcleaner # via -r requirements/edx/base.txt --e . - # via -r requirements/edx/base.txt -e git+https://github.com/openedx/RateXBlock.git@2.0.1#egg=rate-xblock # via -r requirements/edx/base.txt -e git+https://github.com/openedx/xblock-google-drive.git@2d176468e33c0713c911b563f8f65f7cf232f5b6#egg=xblock-google-drive diff --git a/scripts/ci-runner.Dockerfile b/scripts/ci-runner.Dockerfile index f6e4c26e899c..e9acdeb34aed 100644 --- a/scripts/ci-runner.Dockerfile +++ b/scripts/ci-runner.Dockerfile @@ -42,7 +42,6 @@ FROM base as build # Install Python requirements COPY setup.py setup.py -COPY common/lib/ common/lib/ COPY openedx/core/lib openedx/core/lib COPY lms lms COPY cms cms diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh index 202591e31639..e674f13d0b6e 100755 --- a/scripts/circle-ci-tests.sh +++ b/scripts/circle-ci-tests.sh @@ -36,7 +36,7 @@ if [ "$CIRCLE_NODE_TOTAL" == "1" ] ; then echo "To run in more containers, configure parallelism for this repo's settings " echo "via the CircleCI UI and adjust scripts/circle-ci-tests.sh to match." - echo "Running tests for common/lib/ and pavelib/" + echo "Running tests for pavelib/" paver test_lib --cov-args="-p" || EXIT=1 echo "Running python tests for Studio" paver test_system -s cms --cov-args="-p" || EXIT=1 @@ -82,7 +82,7 @@ else paver test_system -s cms --cov-args="-p" ;; - 3) # run the commonlib unit tests + 3) # run the pavelib unit tests paver test_lib --cov-args="-p" ;; diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh index c850ec96b50a..eddc29879183 100755 --- a/scripts/generic-ci-tests.sh +++ b/scripts/generic-ci-tests.sh @@ -17,9 +17,9 @@ set -e # - "lms-unit": Run the LMS Python unit tests # - "cms-unit": Run the CMS Python unit tests # - "js-unit": Run the JavaScript tests -# - "commonlib-unit": Run Python unit tests from the common/lib directory -# - "commonlib-js-unit": Run the JavaScript tests and the Python unit -# tests from the common/lib directory +# - "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 # - "bok-choy": Run acceptance tests that use the bok-choy framework # # `SHARD` is a number indicating which subset of the tests to build. @@ -147,7 +147,7 @@ case "$TEST_SUITE" in exit $EXIT ;; - "lms-unit"|"cms-unit"|"commonlib-unit") + "lms-unit"|"cms-unit"|"pavelib-unit") $TOX bash scripts/unit-tests.sh ;; @@ -156,7 +156,7 @@ case "$TEST_SUITE" in $TOX paver diff_coverage ;; - "commonlib-js-unit") + "pavelib-js-unit") $TOX paver test_js --coverage --skip-clean || { EXIT=1; } paver test_lib --skip-clean $PAVER_ARGS || { EXIT=1; } diff --git a/scripts/post-pip-compile.sh b/scripts/post-pip-compile.sh deleted file mode 100755 index bdf58b60078c..000000000000 --- a/scripts/post-pip-compile.sh +++ /dev/null @@ -1,48 +0,0 @@ -#!/usr/bin/env bash -set -e - -# Remove any cruft from a requirements file generated by pip-compile which we don't want to keep - -function show_help { - echo "Usage: post-pip-compile.sh file ..." - echo "Remove any cruft left behind by pip-compile in the given requirements file(s)." - echo "" - echo "Removes \"-e\" prefixes which were added to GitHub URLs only so that" - echo "pip-compile could process them correctly." -} - -function clean_file { - FILE_PATH=$1 - TEMP_FILE=${FILE_PATH}.tmp - # Workaround for https://github.com/jazzband/pip-tools/issues/204 - - # change absolute paths for local editable packages back to relative ones - FILE_CONTENT=$(<${FILE_PATH}) - FILE_URL_REGEX="-e (file:///[^"$'\n'"]*)/common/lib/\w+" - if [[ "${FILE_CONTENT}" =~ ${FILE_URL_REGEX} ]]; then - BASE_FILE_URL=${BASH_REMATCH[1]} - sed "s|$BASE_FILE_URL/||" ${FILE_PATH} > ${TEMP_FILE} - mv ${TEMP_FILE} ${FILE_PATH} - sed "s|$BASE_FILE_URL|.|" ${FILE_PATH} > ${TEMP_FILE} - mv ${TEMP_FILE} ${FILE_PATH} - fi - # Code sandbox local package installs must be non-editable due to file - # permissions issues. edxapp ones must stay editable until assorted - # packaging bugs are fixed. - if [[ "${FILE_PATH}" == "requirements/edx-sandbox/py38.txt" ]]; then - sed "s|-e common/lib/|common/lib/|" ${FILE_PATH} > ${TEMP_FILE} - mv ${TEMP_FILE} ${FILE_PATH} - fi -} - -for i in "$@"; do - case ${i} in - -h|--help) - # help or unknown option - show_help - exit 0 - ;; - *) - clean_file ${i} - ;; - esac -done diff --git a/scripts/unit-tests.sh b/scripts/unit-tests.sh index d97ad929bd80..51275112764e 100755 --- a/scripts/unit-tests.sh +++ b/scripts/unit-tests.sh @@ -15,7 +15,7 @@ set -e # # - "lms-unit": Run the LMS Python unit tests # - "cms-unit": Run the CMS Python unit tests -# - "commonlib-unit": Run Python unit tests from the common/lib directory +# - "pavelib-unit": Run Python unit tests from the pavelib/paver_tests directory # # `SHARD` is a number indicating which subset of the tests to build. # @@ -109,27 +109,27 @@ case "${TEST_SUITE}" in esac ;; - "commonlib-unit") + "pavelib-unit") case "$SHARD" in "all") - paver test_lib --disable_capture ${PAVER_ARGS} ${PARALLEL} 2> common-tests.log - mv reports/${TEST_SUITE}.coverage reports/.coverage.commonlib + paver test_lib --disable_capture ${PAVER_ARGS} ${PARALLEL} 2> pavelib-tests.log + mv reports/${TEST_SUITE}.coverage reports/.coverage.pavelib ;; [1-2]) - paver test_lib -l ./xmodule --disable_capture --eval-attr="shard==$SHARD" ${PAVER_ARGS} 2> common-tests.${SHARD}.log - mv reports/${TEST_SUITE}.coverage reports/.coverage.commonlib.${SHARD} + paver test_lib -l ./xmodule --disable_capture --eval-attr="shard==$SHARD" ${PAVER_ARGS} 2> pavelib-tests.${SHARD}.log + mv reports/${TEST_SUITE}.coverage reports/.coverage.pavelib.${SHARD} ;; 3|"noshard") - paver test_lib --disable_capture --eval-attr="shard>=$SHARD or not shard" ${PAVER_ARGS} 2> common-tests.3.log - mv reports/${TEST_SUITE}.coverage reports/.coverage.commonlib.3 + paver test_lib --disable_capture --eval-attr="shard>=$SHARD or not shard" ${PAVER_ARGS} 2> pavelib-tests.3.log + mv reports/${TEST_SUITE}.coverage reports/.coverage.pavelib.3 ;; *) # If no shard is specified, rather than running all tests, create an empty xunit file. This is a # backwards compatibility feature. If a new shard (e.g., shard n) is introduced in the build # system, but the tests are called with the old code, then builds will not fail because the # code is out of date. Instead, there will be an instantly-passing shard. - mkdir -p reports/common - emptyxunit "common/nosetests" + mkdir -p reports/pavelib + emptyxunit "pavelib/nosetests" ;; esac ;; diff --git a/scripts/verify-dunder-init.sh b/scripts/verify-dunder-init.sh index 7c88feffc6d3..3c94785c06b9 100755 --- a/scripts/verify-dunder-init.sh +++ b/scripts/verify-dunder-init.sh @@ -30,15 +30,6 @@ exclude+='^\.$' exclude+='|^xmodule/capa/safe_exec/tests/test_files/?.*$' exclude+='|^common/test/data/?.*$' -# Exclude common/lib and its immediate child directories. -# They are not Python packages, but instead are Python sub-projects (hence the setup.py -# in each chlid directory). -# However, we do NOT want to exclude the source directories *within* the sub-projects. -# Example: -# * common/lib/capa -> EXCLUDE from check. -# * common/lib/capa/capa/safe_exec -> INCLUDE in check. -exclude+='|^common/lib$' - # xmodule data folder exclude+='|^xmodule/tests/data/xml-course-root/capa$' exclude+='|^xmodule/tests/data/xml-course-root/uploads/python_lib_zip$' diff --git a/scripts/xdist/get_worker_test_list.py b/scripts/xdist/get_worker_test_list.py index 73910489c681..268ab5ca8915 100644 --- a/scripts/xdist/get_worker_test_list.py +++ b/scripts/xdist/get_worker_test_list.py @@ -25,7 +25,7 @@ @click.option( '--test-suite', help="Test suite that the pytest worker ran.", - type=click.Choice(['lms-unit', 'cms-unit', 'commonlib-unit']), + type=click.Choice(['lms-unit', 'cms-unit']), required=True ) def main(log_file, test_suite): @@ -38,9 +38,6 @@ def main(log_file, test_suite): if worker_num_string not in worker_test_dict: worker_test_dict[worker_num_string] = [] test = regex_search.group(3) - if test_suite == "commonlib-unit": - if "pavelib" not in test and not test.startswith('scripts'): - test = f"common/lib/{test}" worker_test_dict[worker_num_string].append(test) output_folder_name = "worker_list_files" diff --git a/xmodule/modulestore/tests/utils.py b/xmodule/modulestore/tests/utils.py index 78e2760ab291..de0edc1727c6 100644 --- a/xmodule/modulestore/tests/utils.py +++ b/xmodule/modulestore/tests/utils.py @@ -87,9 +87,8 @@ def remove_temp_files_from_list(file_list, dir): # lint-amnesty, pylint: disabl class MixedSplitTestCase(ModuleStoreTestCase): """ - A minimal version of ModuleStoreTestCase for testing in common/lib/ that sets up MixedModuleStore and Split (only). - - It also enables "draft preferred" mode, like Studio uses. + A minimal version of ModuleStoreTestCase for testing in xmodule/modulestore that sets up MixedModuleStore + and Split (only). It also enables "draft preferred" mode, like Studio uses. Draft/old mongo modulestore is not initialized. """