search

asyrjasalo / RESTinstance

Robot Framework library for RESTful JSON APIs
https://pypi.org/project/RESTinstance
GNU Lesser General Public License v3.0
205 stars 84 forks source link

Implemented unset_header #82

Closed kmesserig closed 4 years ago

kmesserig commented 5 years ago

To do several tests in a row with existing and missing headers like authorization, unset_header will remove a list of existing headers.

asyrjasalo commented 5 years ago

I think this PR has potential to shake my "25 keywords at maximum in a library" opinion...

Add some well placed atests and the keyword documentation - particular if there are differences to using Set Headers - and then let's get 1.0.2 out.

kmesserig commented 5 years ago

Sure good point with the 25 keywords. Maybe it would make more sense to support this feature in the Set Headers keyword it self. Because as of now there was no way to remove the headers from the requests.

asyrjasalo commented 5 years ago

Yes, that's why I think Unset Headers would be much useful addition too.

I don't think having 25 keywords as a hard limit makes sense if we can have useful keywords for real life use cases. More like, I rather would not start including a separate keyword for each custom HTTP API authentication though, if we can find a generic way to tackle that with a single keyword instead.

I think the original reasoning was to not bloat the library keyword documentation too much to find the correct keyword as fast as possible. But this will get much easier in any case when editors/IDEs will get better support via https://microsoft.github.io/language-server-protocol/ for Robot Framework.

Having some documentation and tests is better than no documentation and tests. Otherwise this looks good.

kmesserig commented 5 years ago

Closing this pull request due to some library blow up issues as discussed.

asyrjasalo commented 5 years ago

Pardon, I did not get. What you mean by blow up issues?

Tset-Noitamotua commented 5 years ago

I guess Anssi did not meant that this one should be closed, did you? @anssi

What about a clear option that would default to false so that when u want unset headers u do:

Set Headers clear=True

?

asyrjasalo commented 4 years ago

Closing this pull request due to lack of tests and documentation.