search

svanoort / pyresttest

Python Rest Testing
Apache License 2.0
1.15k stars 325 forks source link

Add "cookiejar" option to config. #180

Open danielatdattrixdotcom opened 8 years ago

danielatdattrixdotcom commented 8 years ago

A minor change to use the curl cookiejar and treat one file as a session. I needed this for a Tastypie API using SessionAuthentication. And a white space trimming courtesy of PyCharm.

svanoort-jenkins commented 8 years ago

Can one of the admins verify this patch?

svanoort commented 8 years ago

@danielatdattrixdotcom Thank you for your contribution - this is great. I have to ask, why did you close the PR? I was planning to test it out and merge once I get out of crunch mode at work and have a little time to review.

Cheers, Sam

svanoort-jenkins commented 8 years ago

Can one of the admins verify this patch?

danielatdattrixdotcom commented 8 years ago

@svanoort I ran into an issue it created in not resetting the curl object to a base state so I closed it. The issue presents if you have a PUT then a POST. It needs code to reset the curl object to a ready state without wiping the cookiejar. I'll work out an alternate PR since this one is flawed.

svanoort commented 8 years ago

@danielatdattrixdotcom Probably because it's using CookieJar and CookieFile on the same file, which makes it r/w. One can unset the CookieJar opt to render it read-only, but then you lose the advantage of a persistent session after initial auth.

Considering this use case, would it not be better to create an Extractor that gets the cookies (there's already issue #144 to add extractors geared to cookie operations with the next release), and save cookies to a variable? The cookies are already available on the headers (which all extractors get to play with), there's just not a binding to handle them specially yet.

danielatdattrixdotcom commented 8 years ago

@svanoort Just posted an updated commit with very minor adjustments. I'm writing tests for a project now, so if I encounter any issues I will adjust as-needed if I find I created a problem. I read through #144 , but since we've got curl and it already has mechanisms for managing cookies I did not see a need to reinvent it and add any more complex configuration setup for the simple use of cookies as a test session.

danielatdattrixdotcom commented 8 years ago

@svanoort I ran into a segfault during testing and made one more adjustment.

svanoort commented 8 years ago

@danielatdattrixdotcom I do apologize for the delay (crunch mode at the day job) -- do you consider this to be in a state where I can hit it with the full integrated tests and merge if ready?

danielatdattrixdotcom commented 8 years ago

@svanoort Yes, I think so.