search

mojolingo / sippy_cup

Create SIP load test scenarios the easy way
http://mojolingo.github.io/sippy_cup
MIT License
218 stars 78 forks source link

Add a :headers option to invite #23

Closed wdrexler closed 10 years ago

wdrexler commented 11 years ago

Sometimes, invites need extra headers (like Route:) so let's add an option to the #invite method to do just that.

wdrexler commented 11 years ago

dede06a should have you covered. Unit tested now for great justice.

wdrexler commented 11 years ago

okie dokie, I'm on it!

benlangfeld commented 11 years ago

Should this really take a string, or would a collection of some sort be better? We can't use a hash because SIP messages can have multiple similarly named headers, but some sort of extra structure might be nice.

bklang commented 11 years ago

Should this really take a string, or would a collection of some sort be better? We can't use a hash because SIP messages can have multiple similarly named headers, but some sort of extra structure might be nice.

A collection would be better, yes. However I'm trying to balance the ability to do everything with keeping the YAML simple enough for common cases. I think Will has a proposal to use nested YAML to supply arguments that will be acceptable. See https://github.com/wdrexler/sippy_cup/tree/feature/json_opts

bklang commented 10 years ago

What's the status on this?

benlangfeld commented 10 years ago

This should probably wait until #34 is merged in a couple of hours.

wdrexler commented 10 years ago

Yep, let's wait for the update and I'll make any changes if needed.

benlangfeld commented 10 years ago

Ok, #34 was merged. Please ensure this PR complies with the standards of testing and documentation now in effect on Scenario.

wdrexler commented 10 years ago

I like what you've done with it. I think now testing and docs should be up to standard. Let me know if there are any issues.

wdrexler commented 10 years ago

Sorry about that, I'm still new to generated docs, and I think the duplicate line was a side-effect of a merge.

wdrexler commented 10 years ago

Hmm... those tests weren't broken locally, fixing.

wdrexler commented 10 years ago

That should do it, @benlangfeld.