-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: run integration tests outside LMS container #4
base: main
Are you sure you want to change the base?
Conversation
43ba8c0
to
89cac69
Compare
60e82aa
to
a016821
Compare
Note to reviewers: I'm working on updating the README |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@magajh, This looks great! thank you!
Just to note, when adding these changes we must modify all eox's that use this action, as the commands in your scripts will not work.
description: "Optional extra pip requirements to install in Open edX. E.g: 'package1==1.0 package2>=2.0'" | ||
required: false | ||
default: "" | ||
python_version: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which cases would this input (python_version
) be useful?
echo "Copying fixtures file to the LMS container" | ||
tutor local exec lms mkdir -p /openedx/fixtures | ||
tutor local dc cp ${{ inputs.fixtures_file }} lms:/openedx/fixtures/fixture.json | ||
tutor local exec lms python manage.py lms loaddata /openedx/fixtures/fixture.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is copying better than mounting the file directly with tutor mounts ... ?
if [ ! -d ".venv" ]; then | ||
echo "Virtual environment creation failed" | ||
exit 1 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has the venv creation failed before? I can't think of why it could fail, but there may be edge cases I haven't considered.
Also, should we create the environment before installing any dependencies like tutor?
cat << 'EOF' > plugins/patches.yml | ||
|
||
name: patches | ||
patches: | ||
caddyfile: | | ||
{$default_site_port} { | ||
import proxy "lms:8000" | ||
} | ||
openedx-cms-production-settings: | | ||
ALLOWED_HOSTS = ["*"] | ||
ALLOWED_AUTH_APPLICATIONS = ['cms-sso', 'cms-sso-dev'] | ||
openedx-lms-production-settings: | | ||
ALLOWED_HOSTS = ["*"] | ||
ALLOWED_AUTH_APPLICATIONS = ['cms-sso', 'cms-sso-dev'] | ||
EOF | ||
tutor plugins enable patches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this file to a Caddyfile outside the action definition, or would that be overkill? Composite actions can access their files, so maybe we can leverage that, although it might be more complex for little gain.
Overview
This PR introduces significant modifications to the GitHub Action for testing Open edX plugins in the Tutor environment. These changes are necessary for improving the effectiveness and reliability of our integration tests
Motivation for Changes
The initial implementation ran integration tests inside the LMS container of Tutor. This approach created several challenges:
To address these issues, this PR modifies the workflow to run integration tests outside the Tutor container, improving the testing environment and results.
Workflow Steps and Their Necessity
Set Tutor Environment Variables:
LMS_HOST
,CMS_HOST
, and paths for Tutor and plugins. These variables are essential for proper configuration and operation of Tutor.Install and Prepare Tutor:
Configure Caddyfile and Open edX Settings:
patches.yml
file to customize the Caddyfile and Open edX production settings. This configuration allows for the necessary routing for testing with multiple sites and security settings for our tests.Install Plugin and Extra Requirements:
Run Migrations:
Load Initial Data for the Tests:
Curl Heartbeat:
Run Integration Tests:
Personalization Options
This workflow includes several inputs that can be customized:
app_name
: The name of the application to test (e.g.,eox-core
).tutor_version
: The version of Tutor to use, allowing for testing against different Tutor releases.shell_file_to_run
: The path to the shell script that contains the integration tests.openedx_extra_pip_requeriments
: Optional additional dependencies needed for testing.fixtures_file
: Optional path to a JSON file containing initial data to load for the tests.Testing the Workflow
Steps to test this workflow:
You can create a branch from any of the EOX plugins. For example, create a branch from bav/add-integration-tests in eox-core. I’m using this branch as an example because it includes several integration tests.
Add the script that installs the requirements and runs the tests, for example:
Ensure that the workflow calls the action with the corresponding inputs and uses the branch
mjh/run-integration-tests-outside-container
. Here’s an example of the file that executes the workflow:Check that on-push, the integration tests run successfully, as shown in this test branch: test: add integration tests eox-core#278
⭐ Important:
This PR needs to be updated with the implementation from PR #3 to support running the tests in Tutor nightly. The update will be made once PR #3 is merged.