-
Notifications
You must be signed in to change notification settings - Fork 2
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
🐛(tests) configure DRF in lms settings & use lms settings in tests #9
Conversation
16ed908
to
3125e5c
Compare
3125e5c
to
dfb46ca
Compare
RUN if [ ${user} -ne 0 -a ${group} -ne 0 ]; then \ | ||
groupadd --gid $group app ; \ | ||
useradd --uid $user --gid $group --home /app app ; \ | ||
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.
use redhat pattern as we don't want to create images that work only with one user?
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.
If you don't mind I would prefer to handle this in a new PR as it requires substantial changes. I'll declare a new issue for this. Deal?
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.
FTR #11
- CI_STEP=quality | ||
- CI_STEP=docs | ||
- CI_STEP=test | ||
- CI_STEP=test-spec OPENEDX_RELEASE=ginkgo.1 |
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.
I thought you were removing Travis?
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.
I will. Not in this PR. I'll declare another issue for this. Deal?
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.
FTR #10
echo "Running tox:$TOXENV test suite..." | ||
# Perform database migrations | ||
echo "Starting database migration" | ||
_dc_run lms python ./manage.py lms migrate |
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.
cms migrations?
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.
Fonzie is installed in the LMS application. Is the LMS standalone, or does it requires the CMS to run?
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.
ok so maybe remove it from elsewhere?
bin/ci
Outdated
;; | ||
test) | ||
make test | ||
;; |
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.
Line 95 -> 103 ca be replaced by
docs|quality|test)
make $CI_STEP
;;
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.
Nice catch! Thx!
** FIX DETAILS ** * configure & test DRF API versioning * use lms settings in tests * get rid of tox in our CI: Using tox within docker containers seems overkill; let's make things a little bit "simpler" by dropping it once for all. * Prevent root user creation: when trying to build the base container with no user or group as build arguments, docker tries to create the root user. We prevent this behavior by only creating a user when uid:gid are distinct from root user ids. * Update volume endpoints: fundocker/edxapp now use the same application tree as edx, we need to stick to it to make it works. ** MISC ** * fix broken comparison in ci script * use super() in test cases setUp method * remove unused CMS service * improve CI step switch/case
13edc15
to
c0980f4
Compare
Purpose
We need to use lms settings in our tests so that we are in an Open edX context.
Proposal
tox