-
Notifications
You must be signed in to change notification settings - Fork 31
adding volume #35
adding volume #35
Conversation
garethr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @luckyraul. The code for this looks good. At the moment the tests are failing as the assertions are against the wrong context attributes, but that should just be a matter of fixing the wording indicated in the comments. Thanks for adding too.
| volume: '/var/www', | ||
| } | ||
| end | ||
| it 'should expand the port to an array' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should refer to the volume rather than the port
| } | ||
| end | ||
| it 'should expand the port to an array' do | ||
| expect(context).to include(expose: ['/var/www']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should make an assertion against the volume attribute rather than expose
| volume: '/var/www,/var/lib', | ||
| } | ||
| end | ||
| it 'should expand the labels to an array' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should refer to volume rather than the labels
| } | ||
| end | ||
| it 'should expand the labels to an array' do | ||
| expect(context[:expose]).to include('/var/www','/var/lib') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above this should refer to volume rather than expose
|
@luckyraul neat addition. Thanks again. |
No description provided.