pylti / lti

Learning Tools Interoperability for Python
Other
78 stars 45 forks source link

Fix attribute name #44

Closed michaelwheeler closed 7 years ago

michaelwheeler commented 7 years ago

The HttpRequest attribute is body not data. https://docs.djangoproject.com/en/1.11/ref/request-response/#django.http.HttpRequest.body

codecov[bot] commented 7 years ago

Codecov Report

Merging #44 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #44   +/-   ##
=======================================
  Coverage   82.34%   82.34%           
=======================================
  Files          15       15           
  Lines         623      623           
  Branches      111      111           
=======================================
  Hits          513      513           
  Misses         82       82           
  Partials       28       28
Impacted Files Coverage Δ
src/lti/outcome_request.py 64.91% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 83caeb3...fd78351. Read the comment docs.

michaelwheeler commented 7 years ago

Great! I looked into adding tests, but was a little fuzzy about how to proceed. Do add django as a dep in tox.ini and put my test in test_outcome_request.py? If so I'll take a stab at it.

ryanhiebert commented 7 years ago

That seems appropriate, though we may want to consider splitting different LMS tests up into different Tox runs. I'm not sure about that, but I think we want to make sure we don't have weird cross-dependencies on different frameworks that cause bugs.

Testing is not a solved problem for us yet, so find small improvements, and open pull requests for them. The smaller the better, to avoid delays where we're not yet on the same page about some other part of it. Bigger changes are fine, but they will take more time for me to approve.