Skip to content

Commit

Permalink
refactor!: delete common/lib and related usages
Browse files Browse the repository at this point in the history
  • Loading branch information
UsamaSadiq committed Sep 22, 2022
1 parent 96f6174 commit 897cb36
Show file tree
Hide file tree
Showing 40 changed files with 92 additions and 228 deletions.
1 change: 0 additions & 1 deletion .coveragerc-local
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ data_file = reports/.coverage
source =
cms
common/djangoapps
common/lib/xmodule
lms
openedx
pavelib
Expand Down
3 changes: 0 additions & 3 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion .github/actions/verify-tests-count/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-static-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion .github/workflows/docs-build-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/js-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node-version }}

- name: Setup npm
run: npm i -g [email protected]

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/migrations-check-mysql8.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/migrations-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pylint-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/quality-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
os: [ ubuntu-20.04 ]
python-version: [ 3.8 ]
node-version: [ 16 ]

steps:

- uses: actions/checkout@v2
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/static-assets-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/unit-tests-gh-hosted.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.

This comment has been minimized.

Copy link
@timmc-edx

timmc-edx Oct 3, 2022

Contributor

I believe this creates a syntax error. Not sure how to test this workflow, though.

This comment has been minimized.

Copy link
@timmc-edx

This comment has been minimized.

Copy link
@kdmccormick

kdmccormick Oct 5, 2022

Member

@timmc-edx I just noticed the same thing. I'm pretty sure the issue is that the # is not aligned with the rest of the text block. I can test this by opening a PR from my personal edx-platform fork.

This comment has been minimized.

Copy link
@kdmccormick

kdmccormick Oct 5, 2022

Member

@UsamaSadiq @timmc-edx Here's a fix (assuming tests pass): #31105

pip install -e .
pip install "django~=${{ matrix.django-version }}.0"

- name: Setup and run tests
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/verify-gha-unit-tests-count.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
14 changes: 3 additions & 11 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
18 changes: 15 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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
Expand Down
29 changes: 0 additions & 29 deletions common/lib/conftest.py

This file was deleted.

22 changes: 0 additions & 22 deletions common/lib/pytest.ini

This file was deleted.

2 changes: 1 addition & 1 deletion docs/decisions/0013-library-reference-content-block.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
9 changes: 0 additions & 9 deletions docs/guides/docstrings/common_lib.rst

This file was deleted.

2 changes: 2 additions & 0 deletions pavelib/prereqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
14 changes: 7 additions & 7 deletions pavelib/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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={
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -197,28 +197,28 @@ 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,
passthrough_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,
Expand Down
10 changes: 2 additions & 8 deletions pavelib/utils/envs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Loading

0 comments on commit 897cb36

Please sign in to comment.