biocore / labadmin

Administration website for the Knight Lab
4 stars 16 forks source link

added some more tests #137

Closed sjanssen2 closed 7 years ago

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.863% when pulling fac0b593918f728ed1196a947752661d42e2ee32 on sjanssen2:unittest_ag_pulldown into 7c5976702169c81defd5bcc151255d212d61c050 on biocore:master.

wasade commented 8 years ago

I have no idea, new to multipart form data. Relevant code here:

https://github.com/biocore/labadmin/blob/master/knimin/tests/tornado_test_base.py#L85

On Mon, Oct 3, 2016 at 1:34 PM, Stefan Janssen notifications@github.com wrote:

@sjanssen2 commented on this pull request.

In knimin/tests/test_ag_pulldown.py https://github.com/biocore/labadmin/pull/137:

@@ -73,6 +73,11 @@ def test_post(self): ' for file download. It may take a while with many ' 'barcodes.'), response.body)

  • data = {'external': 'ab', 'external': 'cd'}

That was also my first idea, but I get the following error:

E

ERROR: test_post (main.testAGPulldownHandler)

Traceback (most recent call last): File "knimin/tests/test_ag_pulldown.py", line 77, in test_post response = self.multipart_post('/ag_pulldown/', data, files) File "/media/barnacle/home/sjanssen/AmericanGut/labadmin/knimin/tests/tornado_test_base.py", line 72, in multipart_post file_info) File "/media/barnacle/home/sjanssen/AmericanGut/labadmin/knimin/tests/tornado_test_base.py", line 121, in encode_multipart_formdata body = CRLF.join(L) TypeError: sequence item 3: expected string, list found


Ran 1 test in 0.093s

FAILED (errors=1)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/biocore/labadmin/pull/137, or mute the thread https://github.com/notifications/unsubscribe-auth/AAc8su5NvN3XJGZyyY2p1G9r1RTrhwX_ks5qwVjPgaJpZM4KG7vF .

sjanssen2 commented 8 years ago

@wasade I don't understand why Kiwi's comments show up here? I have the feeling that I did something wrong to git :-/ Could you please check that I did not break anything?!

josenavas commented 8 years ago

@sjanssen2 it looks like you merged Kiwi's branch into your branch - since Kiwi's branch is not in master, we can't merge this as this will include his work... I'm not sure which is the best way to fix it, but creating a different branch, copying the files that you want and doing a PR with that other branch may be your best bet (you can also check with @ElDeveloper to see if there is a workaround)

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.3%) to 90.052% when pulling 250fdc3e26807624b4a259c0493552fda7b19e3f on sjanssen2:unittest_ag_pulldown into 7c5976702169c81defd5bcc151255d212d61c050 on biocore:master.

josenavas commented 7 years ago

@sjanssen2, is it possible that the actual information was jsonized? The value will be a single JSON string, so you need to decode it, but I'm unsure if this is how it was done - just pointing this out as an alternative.

2016-11-03 10:00 GMT-07:00 Stefan Janssen notifications@github.com:

@sjanssen2 commented on this pull request.

In knimin/tests/test_ag_pulldown.py https://github.com/biocore/labadmin/pull/137:

@@ -73,6 +73,11 @@ def test_post(self): ' for file download. It may take a while with many ' 'barcodes.'), response.body)

  • data = {'external': 'ab', 'external': 'cd'}

Having read some more of the tornado code, I think it is not possible to pass a list of something (here str) to a post request and obtain it natively as a list. Thus, the if statement is misleading in lines 37 - 40 in 'ag_pulldown.py' since 'external' is either the empty string or the single value passed to the function, but never something with list-like separated by ","

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/biocore/labadmin/pull/137, or mute the thread https://github.com/notifications/unsubscribe-auth/ACYrZpmuv3h7bUmZyXJp1rXQBejxQ5eIks5q6hNHgaJpZM4KG7vF .

Jose Navas

sjanssen2 commented 7 years ago

Hey @josenavas , good point. However, I tired that json route already without success.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 91.772% when pulling 147e9f7ec5df8ae939bb0b0fd7eda4254088197e on sjanssen2:unittest_ag_pulldown into c12fd3c69231ae2a8b945705d6ed1c4a8f3073ee on biocore:master.

sjanssen2 commented 7 years ago

I want to get this off my table, @mortonjt could you help with a review?

sjanssen2 commented 7 years ago

I think the todo is of no major concern, it is more about code coverage. Maybe we can refactor the code later, but I would not bother people with raising an issue for this one.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 91.772% when pulling e71752194e68452d14f2a7c024a8fb42a58ab494 on sjanssen2:unittest_ag_pulldown into c12fd3c69231ae2a8b945705d6ed1c4a8f3073ee on biocore:master.

sjanssen2 commented 7 years ago

@wasade ready to merge?