Use a template library to render test xml #6

Merged
merged 10 commits into from Jan 24, 2013

Projects

None yet

3 participants

@jlward
Member
jlward commented Jan 21, 2013

No description provided.

@winhamwr winhamwr commented on an outdated diff Jan 23, 2013
docx2html/tests/__init__.py
- <v:shape id="_x0000_i1027" type="#_x0000_t75" style="width:99.75pt;height:116.25pt">
- </v:shape>
- </w:pict>
- </w:r>
- </w:p>
-""".strip()
+env = Environment(
+ loader=PackageLoader(
+ 'docx2html.tests',
+ 'templates',
+ ),
+)
+
+
+def create_xml(body):
+ template = env.get_template('base.xml')
@winhamwr
winhamwr Jan 23, 2013 PolicyStat LLC member

I would prefer a dictionary of constants for all of these template strings, since some of them are used multiple times (p.xml for example).

@winhamwr winhamwr commented on an outdated diff Jan 23, 2013
docx2html/tests/templates/tr.xml
@@ -0,0 +1,8 @@
+<w:tr>
+ <w:trPr>
+ <w:cantSplit w:val="false"/>
+ </w:trPr>
+ {% for tc in tcs %}
@winhamwr
winhamwr Jan 23, 2013 PolicyStat LLC member

Can we use some better variable names for these templates things rather than abbreviations? I know that has some meaning to you, but it's kind of rough.

@winhamwr winhamwr commented on an outdated diff Jan 23, 2013
docx2html/tests/__init__.py
- <w:color w:val="000000"/>
- </w:rPr>
- </w:pPr>
- <w:r w:rsidR="00360165">
- <w:rPr>
- <w:b/>
- <w:color w:val="000000"/>
- </w:rPr>
- <w:pict>
- <v:shape id="_x0000_i1027" type="#_x0000_t75" style="width:99.75pt;height:116.25pt">
- </v:shape>
- </w:pict>
- </w:r>
- </w:p>
-""".strip()
+env = Environment(
@winhamwr
winhamwr Jan 23, 2013 PolicyStat LLC member

What do you think about moving all of these generators to a DocxBuilder class. You could then use that to make it obvious what they were doing and drop the create_ prefix. Then when in the test files, you do something like:

from docx2html.tests import DocxBuilder as DXB

DXB.table(...)
DXB.p_tag(..)

That lets you be more concise while also making it obvious which functions are just throwing out XML and from where they're coming.

@winhamwr winhamwr and 1 other commented on an outdated diff Jan 23, 2013
@@ -29,7 +29,7 @@ def get_readme():
packages=find_packages(),
scripts=[],
zip_safe=False,
- install_requires=['lxml==2.2.4', 'pillow==1.7.7'],
+ install_requires=['lxml==2.2.4', 'pillow==1.7.7', 'Jinja2==2.6'],
@winhamwr
winhamwr Jan 23, 2013 PolicyStat LLC member

Shouldn't this just go in test_requirements.txt? If you're having trouble because you have a jinja import in an __init__.py, you could either move the helper code to a tests.docx_builder.py module or do something like:

try:
    import Jinja2
except ImportError:
    Jinja2 = None

I'd say moving the helpers to a different module would probably be cleaner, since you would get an obvious error message when someone tried to run the tests without jinja.

@jlward
jlward Jan 23, 2013 PolicyStat LLC member

This is residual from when I did not know where I was going to put the template files, I will move it.

@winhamwr winhamwr commented on an outdated diff Jan 24, 2013
docx2html/tests/__init__.py
@@ -1,222 +1,118 @@
-import re
+from jinja2 import Environment, PackageLoader
@winhamwr
winhamwr Jan 24, 2013 PolicyStat LLC member

I think we already talked about this, but since the ticket is marked ready for review, just reminding you to move this stuff to another module.

@winhamwr winhamwr commented on an outdated diff Jan 24, 2013
test_requirements.txt
@@ -1,2 +1,3 @@
nose
mock
+Jinja==2.6
@winhamwr
winhamwr Jan 24, 2013 PolicyStat LLC member

Does the requirement need to be pegged that specifically? Seems like say >=2.0 would probably be good enough?

@kylegibson
Member

you can actually put these on separate lines, like this:

install:
   - python setup.py -q install
   - pip install -r test_requirements.txt

See https://github.com/PolicyStat/terrarium/blob/master/.travis.yml

@jlward jlward merged commit 25991f6 into master Jan 24, 2013

1 check passed

Details default The Travis build passed
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #6: updated tests to use jinja templates 57f9757
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #6: added jinja to the requirements f4ffa5e
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #6: stopped using abbreviated names d6c6ff9
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #6: moved req to test req only 514fcaa
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #6: looser reqs for jinja 361799d
@jlward jlward added a commit that referenced this pull request Mar 14, 2014
@jlward jlward refs #6: lets tell travis how to install test reqs for real; install …
…the correct version of jinja(2)
bccc063
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment