Coerce default_user, default_pass, exchange and vhost to binaries

[priviterag]

fix #25

do we need integration tests?

[michaelklishin]

No, I don't think we need integration tests, this is a fairly trivial change.

[michaelklishin]

While we are at it, can be do the same for default_user and default_password? Those must be binaries as well.

[videlalvaro]

perhaps you could use assertEqual? from eunit instead of just relying on erlang's pattern matching, since according to the docs it can provide more informative error messages

[priviterag]

sure, I will. While we are discussing tests, I want to share my thoughts about this task. Here I'm writing unit tests for a function that is (at least, so far) private (used only by the rabbit_mqtt_util:env/1 function). What I would have preferred is to call rabbit_mqtt_util:env/1 from my tests and check the return value using assertEqual. This is not simple, because env call application:get_env/2 and this would require an instance of rabbitmq running (I guess). My question is: can we use some stub/mock erlang library for this? Are we already using it in other repos?

[michaelklishin]

Your tests are similar to these. To use application:get_env/2 the app needs to be loaded, but not necessarily running. See application:load/1.

[priviterag]

thanks, looking into it.

[priviterag]

found this one: https://github.com/eproxus/meck

[videlalvaro]

yeah, there are mocking tools in Erlang, but without erlang.mk we are not going to be using them, since integrating them would be duplicated work.

Right now you can have an application config file with your mqtt settings, load that with the application module, and then run your tests.

[priviterag]

@michaelklishin jeez, you are a contributor!

[michaelklishin]

@priviterag is this ready for QA?

[priviterag]

@michaelklishin yep, it's ready

[michaelklishin]

@priviterag if a pull request doesn't get any attention in more than 2 days it may be a good idea to explicitly state that it is ready.