Skip to content
This repository has been archived by the owner on Jun 18, 2024. It is now read-only.

Initial implementation #1

Merged
merged 33 commits into from
Dec 10, 2015
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6cb67fc
Add skeleton created by XBlock SDK.
itsjeyd Oct 27, 2015
9a9a324
Allow to display and interact with exercises in the LMS.
itsjeyd Oct 30, 2015
dd06d81
Add support for exercise descriptions.
itsjeyd Oct 30, 2015
aa7c134
Add grading functionality.
itsjeyd Oct 30, 2015
c19b0e4
Save and restore state.
itsjeyd Oct 30, 2015
6c1b897
Clean up.
itsjeyd Oct 30, 2015
6b87e0a
Address review comments.
itsjeyd Nov 2, 2015
c71e96b
Configure Travis.
itsjeyd Nov 2, 2015
a838256
Port tests.
itsjeyd Nov 2, 2015
273232b
Fix PEP8 and pylint issues.
itsjeyd Nov 2, 2015
06d0cbb
Render template for board and menu on the server.
itsjeyd Nov 3, 2015
fbc4ea4
Add workbench scenario.
itsjeyd Nov 3, 2015
06eacfc
Address review comments.
itsjeyd Nov 4, 2015
deac8d0
Add integration tests.
itsjeyd Nov 10, 2015
bc15783
Update test-requirements.
itsjeyd Nov 10, 2015
a67ccc1
Address review comments.
itsjeyd Nov 16, 2015
918ed51
Move controls and vector properties above board.
itsjeyd Nov 20, 2015
b574ec3
Address review comments.
itsjeyd Nov 21, 2015
708b2cc
Address review comments (i18n).
itsjeyd Nov 21, 2015
f19ab78
WYSIWYG functionality for Studio, Part 1: Add support for definining
itsjeyd Nov 18, 2015
c3a098d
Address review comments.
itsjeyd Nov 23, 2015
d2fe045
Make LMS and Studio functionality accessible.
itsjeyd Nov 24, 2015
d2d0e84
Studio: Make exercise preview render correctly on page load.
itsjeyd Nov 25, 2015
224391a
Address review comments.
itsjeyd Nov 26, 2015
602d021
WYSIWYG functionality for Studio, Part 2:
itsjeyd Nov 25, 2015
9e6e001
Address review comments.
itsjeyd Nov 30, 2015
dcf0840
Address review comments.
itsjeyd Dec 1, 2015
792d4ba
Remove :hover effect from disabled buttons.
itsjeyd Dec 2, 2015
d2a9f6e
Address upstream review comments.
itsjeyd Dec 4, 2015
0d4a06a
Address upstream review comments.
itsjeyd Dec 7, 2015
645115f
Address upstream review comments.
itsjeyd Dec 8, 2015
c03551d
Address upstream review comments.
itsjeyd Dec 9, 2015
461371b
Address upstream review comments.
itsjeyd Dec 9, 2015
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
__pycache__/
*.py[cod]
tests.integration.*.log
tests.integration.*.png
vectordraw_xblock.egg-info/
var/
19 changes: 19 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
language: python
python:
- 2.7
before_install:
- export DISPLAY=:99
- sh -e /etc/init.d/xvfb start
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really used yet, since there aren't any integration tests. We can leave this here, since we will have to add integration tests anyway. They will be a bit tricky to implement, but it won't be too bad.

install:
- pip install -r test-requirements.txt
- pip install -r $VIRTUAL_ENV/src/xblock-sdk/requirements/base.txt
- pip install -r $VIRTUAL_ENV/src/xblock-sdk/requirements/test.txt
- pip install -r $VIRTUAL_ENV/src/xblock/requirements.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fragile, because the xblock and xblock-sdk repos don't expect their requirements to be installed like this. I would really like to understand why it's necessary to install these, and what the right way to install them is.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nedbat Requirements files are flat. The transitive requirements of packages listed in requirements.txt are not installed implicitly, unless they are specified in install_requires in setup.py. However, only requirements available on PyPI can be implicitly installed in this way. Since many of our requirements are installed from Github, we can only list them in requirements.txt, and we then have to resort to hacks like this one to install the transitive dependencies.

An alternative would be to flatten the list of recursive requirements manually, and list all of them in requirements.txt in this XBlock. However, this is fragile as well, since then we have to update requirements in even more places in sync.

Uploading all versions of our packages to PyPI comes with its own set of problems, too. I'm sure we could manage requirements in a better way than we currently do, but it isn't an easy problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nedbat Are you saying we should use cd $VIRTUAL_ENV/src/xblock-sdk/ && make pip instead of the commands that we are using now (which are the same as what's in the Makefile)? Or something else?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarnach @bradenmacdonald I don't have a specific proposal. I wanted to know why you needed to install xblock-sdk's test requirements. I would have thought they were only needed for testing the sdk, which you are not doing here. I understand about the difficulty of managing packages, and I'm sure part of this is us being sloppy in the xblock-sdk repo. I wanted to understand the problem so that we could decide on a way to fix it, wherever the best place is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nedbat Our integration tests on Travis CI use bok choy and selenium to run through the various XBlock views/handlers hosted within the XBlock SDK "workbench" runtime. That means that we need to install XBlock, XBlock SDK, bok choy, mock, ddt, etc. But if we specify those requirements in our own XBlock's requirements.txt (whether using pinned versions or not), we'll inevitably arrive at a situation where our XBlock requires a different version of some dependency than xblock-sdk does, which leads to inconsistent test results depending on environment and order of installation. And for people like me who have an "xblock venv" where I install xblock-sdk plus about 20 different XBlocks that I work on, version mismatches can be frustrating time sinks.

So we've found that the easiest solution for now is just to say "we'll have what they're having" with respect to xblock-sdk, and standardize on whatever requirements it is using.

script:
- pep8 --max-line-length=100 vectordraw
- pylint vectordraw
- ./run_tests.py --with-coverage --cover-package=vectordraw
notifications:
email: false
addons:
firefox: 36.0
19 changes: 19 additions & 0 deletions pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[REPORTS]
reports=no

[FORMAT]
max-line-length=100

[MESSAGES CONTROL]
disable=
I,
attribute-defined-outside-init,
maybe-no-member,
star-args,
too-few-public-methods,
too-many-ancestors,
too-many-instance-attributes,
too-many-public-methods

[VARIABLES]
dummy-variables-rgx=_$|dummy|unused
3 changes: 3 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
git+https://github.com/edx/[email protected]#egg=XBlock
git+https://github.com/edx/xblock-utils.git@b4f9b51146c7fafa12f41d54af752b8f1516dffd#egg=xblock-utils
-e .
52 changes: 52 additions & 0 deletions run_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/usr/bin/env python
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file isn't necessary for the unit tests as well, but good to have for the integration tests.

# -*- coding: utf-8 -*-
"""Run tests for the Vector Drawing XBlock.

This script is required to run our selenium tests inside the xblock-sdk workbench
because the workbench SDK's settings file is not inside any python module.
"""

import os
import logging
import sys

from django.conf import settings
from django.core.management import execute_from_command_line

logging_level_overrides = {
'workbench.views': logging.ERROR,
'django.request': logging.ERROR,
'workbench.runtime': logging.ERROR,
}

if __name__ == '__main__':
# Use the workbench settings file:
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'workbench.settings')
# Configure a range of ports in case the default port of 8081 is in use
os.environ.setdefault('DJANGO_LIVE_TEST_SERVER_ADDRESS', 'localhost:8081-8099')

try:
os.mkdir('var')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you making this directory? What is it used for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singingwolfboy The workbench uses the var directory for database and log files. Creating it here to ensure that the tests don't fail because it's missing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fix this in the workbench eventually. It's kind of stupid that workbench simply errors out if the directory doesn't exist, so all XBlocks have to create it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write issues for things like this. I've done this one: https://github.com/edx/xblock-sdk/issues/99

except OSError:
# May already exist.
pass

settings.INSTALLED_APPS += ('vectordraw', )

for noisy_logger, log_level in logging_level_overrides.iteritems():
logging.getLogger(noisy_logger).setLevel(log_level)

args_iter = iter(sys.argv[1:])
options = []
paths = []
for arg in args_iter:
if arg == '--':
break
if arg.startswith('-'):
options.append(arg)
else:
paths.append(arg)
paths.extend(args_iter)
if not paths:
paths = ['tests/']
execute_from_command_line([sys.argv[0], 'test'] + options + ['--'] + paths)
40 changes: 40 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
"""Setup for vectordraw XBlock."""

import os
from setuptools import setup


def package_data(pkg, roots):
"""Generic function to find package_data.

All of the files under each of the `roots` will be declared as package
data for package `pkg`.

"""
data = []
for root in roots:
for dirname, _, files in os.walk(os.path.join(pkg, root)):
for fname in files:
data.append(os.path.relpath(os.path.join(dirname, fname), pkg))

return {pkg: data}


setup(
name='vectordraw-xblock',
version='0.1',
description='vectordraw XBlock', # TODO: write a better description.
packages=[
'vectordraw',
],
install_requires=[
'XBlock',
'xblock-utils',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like pyyaml is missing from this list. It's listed in the requirements.txt file, and not here. (Maybe you should unify these somehow?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singingwolfboy Thanks for catching that. The Vector Drawing XBlock itself actually doesn't make use of pyyaml (it only depends on it because it extends XBlock). So instead of adding it here I removed pyyaml from requirements.txt and bumped the XBlock version to 0.4.2 (which lists pyyaml in its requirements.txt file).

],
entry_points={
'xblock.v1': [
'vectordraw = vectordraw:VectorDrawXBlock',
]
},
package_data=package_data("vectordraw", ["static", "public"]),
)
5 changes: 5 additions & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Django>=1.8, <1.9
-r requirements.txt
-e git+https://github.com/edx/xblock-sdk.git@8eb5f174dc59c0f4e40e10eaab56753958651d17#egg=xblock-sdk
ddt
selenium==2.47.3 # 2.48 is not working atm
Empty file added tests/__init__.py
Empty file.
Empty file added tests/integration/__init__.py
Empty file.
Loading