config cleanup with additional test coverage #168
Conversation
| describe "#ensure!" do | ||
| subject{ Hubspot::Config.ensure!(:hapikey, :base_url, :portal_id)} | ||
| before{ Hubspot::Config.configure(config) } | ||
| describe ".ensure!" do |
There was a problem hiding this comment.
It wasn't necessary to update these tests but I opted to update them to remove the calls to should which prints a deprecation warning:
Using
shouldfrom rspec-expectations' old:shouldsyntax without explicitly enabling the syntax is deprecated. Use the new:expectsyntax or explicitly enable:shouldwithconfig.expect_with(:rspec) { |c| c.syntax = :should }instead.
| Hubspot.configure(hapikey: ENV['HUBSPOT_API_KEY']) unless hapikey.blank? | ||
| end | ||
| Hubspot.configure(hapikey: hapikey) | ||
| yield if block_given? |
There was a problem hiding this comment.
Updating the config to raise when the an authentication approach is not set resulted in a few tests failing because the ensure block is setting the hapikey to a nil value.
This method currently:
sets the hapikey to the provided hapikey unless the argument is blank.
runs the block if a block is given
sets the hapikey to the env variable HUBSPOT_API_KEY unless the hapikey is blank.
I think it's safe to simplify this method to rely on an hapikey value being provided and not attempting to set the hapikey to an environment variable. This method is used by the deprecated methods dump_properties and dump_properties.
| @@ -47,6 +48,11 @@ def ensure!(*params) | |||
| raise Hubspot::ConfigurationError.new("'#{p}' not configured") unless instance_variable_get "@#{p}" | |||
| end | |||
| end | |||
|
|
|||
| private | |||
There was a problem hiding this comment.
How would you feel about adding a newline between this private line and the next method definition?
SViccari
force-pushed
the
sv-config-cleanup-with-additional-test-coverage
branch
2 times, most recently
from
114f7ea to
27d40c4
Compare
Summary: Updates the config to raise when the hapikey or oauth access token is not set or when both values are set.
SViccari
force-pushed
the
sv-config-cleanup-with-additional-test-coverage
branch
from
27d40c4 to
45692fb
Compare
Summary:
Update
Hubspot.configureto raise when the necessary authentication information is not provided or when two ways of authenticating are provided.