Skip to content

Commit

Permalink
Merge pull request #21553 from xclusv/develop-fix-21491
Browse files Browse the repository at this point in the history 
Fix for issue #21491 (composer install should always run)
  • Loading branch information
@thatch45
thatch45 committed Mar 11, 2015
2 parents e3e455e + d15f867 commit aef9701
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 19 deletions.
21 changes: 12 additions & 9 deletions salt/modules/composer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def _valid_composer(composer):

def did_composer_install(dir):
'''
Test to see if the composer.lock file exists in this directory
Test to see if the vendor directory exists in this directory
dir
Directory location of the composer.json file
Expand All @@ -49,7 +49,7 @@ def did_composer_install(dir):
salt '*' composer.did_composer_install /var/www/application
'''
lockFile = "{0}/composer.lock".format(dir)
lockFile = "{0}/vendor".format(dir)
if os.path.exists(lockFile):
return True
return False
Expand All @@ -67,7 +67,8 @@ def _run_composer(action,
optimize=None,
no_dev=None,
quiet=False,
composer_home='/root'):
composer_home='/root',
extra_flags=None):
'''
Run PHP's composer with a specific action.
Expand Down Expand Up @@ -117,6 +118,9 @@ def _run_composer(action,
composer_home
$COMPOSER_HOME environment variable
extra_flags
None, or a string containing extra flags to pass to composer.
'''
if composer is not None:
if php is None:
Expand All @@ -138,15 +142,12 @@ def _run_composer(action,
raise SaltInvocationError('{0!r} is required for {1!r}'
.format('action', 'composer._run_composer'))

# If we're running an update, and if composer.lock does not exist, then
# we really need to run install instead.
if action == 'update':
if not did_composer_install(dir):
action = 'install'

# Base Settings
cmd = '{0} {1} {2}'.format(composer, action, '--no-interaction --no-ansi')

if extra_flags is not None:
cmd = '{0} {1}'.format(cmd, extra_flags)

# If php is set, prepend it
if php is not None:
cmd = php + ' ' + cmd
Expand Down Expand Up @@ -345,6 +346,7 @@ def update(dir,
no_dev=True optimize=True
'''
result = _run_composer('update',
extra_flags='--no-progress',
dir=dir,
composer=composer,
php=php,
Expand Down Expand Up @@ -397,6 +399,7 @@ def selfupdate(composer=None,
salt '*' composer.selfupdate
'''
result = _run_composer('selfupdate',
extra_flags='--no-progress',
composer=composer,
php=php,
runas=runas,
Expand Down
23 changes: 18 additions & 5 deletions salt/states/composer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ def installed(name,
optimize=None,
no_dev=None,
quiet=False,
composer_home='/root'):
composer_home='/root',
always_check=True):
'''
Verify that composer has installed the latest packages give a
``composer.json`` and ``composer.lock`` file in a directory.
Verify that the correct versions of composer dependencies are present.
dir
Directory location of the composer.json file.
Expand Down Expand Up @@ -106,22 +106,35 @@ def installed(name,
composer_home
$COMPOSER_HOME environment variable
always_check
If True, _always_ run `composer install` in the directory. This is the
default behavior. If False, only run `composer install` if there is no
vendor directory present.
'''
ret = {'name': name, 'result': None, 'comment': '', 'changes': {}}

did_install = __salt__['composer.did_composer_install'](name)

# Check if composer.lock exists, if so we already ran `composer install`
# and we don't need to do it again
if __salt__['composer.did_composer_install'](name):
if always_check is False and did_install:
ret['result'] = True
ret['comment'] = 'Composer already installed this directory'
return ret

# The state of the system does need to be changed. Check if we're running
# in ``test=true`` mode.
if __opts__['test'] is True:

if did_install is True:
install_status = ""
else:
install_status = "not "

ret['comment'] = 'The state of "{0}" will be changed.'.format(name)
ret['changes'] = {
'old': 'composer install has not yet been run in {0}'.format(name),
'old': 'composer install has {0}been run in {1}'.format(install_status, name),
'new': 'composer install will be run in {0}'.format(name)
}
ret['result'] = None
Expand Down
19 changes: 14 additions & 5 deletions tests/unit/modules/composer_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,25 @@ def test_install(self):
'''
Test for Install composer dependencies for a directory.
'''

# Test _valid_composer=False throws exception
mock = MagicMock(return_value=False)
with patch.object(composer, '_valid_composer', mock):
self.assertRaises(CommandNotFoundError, composer.install, 'd')

# Test no directory specified throws exception
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
self.assertRaises(SaltInvocationError, composer.install, None)

# Test `composer install` exit status != 0 throws exception
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value={'retcode': 1, 'stderr': 'A'})
with patch.dict(composer.__salt__, {'cmd.run_all': mock}):
self.assertRaises(CommandExecutionError, composer.install, 'd')

# Test success with quiet=True returns True
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value={'retcode': 0, 'stderr': 'A'})
Expand All @@ -58,6 +63,7 @@ def test_install(self):
None, None, None, None, None,
True))

# Test success with quiet=False returns object
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
rval = {'retcode': 0, 'stderr': 'A', 'stdout': 'B'}
Expand All @@ -69,10 +75,13 @@ def test_update(self):
'''
Test for Update composer dependencies for a directory.
'''

# Test _valid_composer=False throws exception
mock = MagicMock(return_value=False)
with patch.object(composer, '_valid_composer', mock):
self.assertRaises(CommandNotFoundError, composer.update, 'd')

# Test no directory specified throws exception
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value=True)
Expand All @@ -88,7 +97,7 @@ def test_update(self):
with patch.dict(composer.__salt__, {'cmd.run_all': mock}):
self.assertRaises(CommandExecutionError, composer.update, 'd')

# Test update with existing composer.lock and quiet=True
# Test update with existing vendor directory and quiet=True
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value=True)
Expand All @@ -99,7 +108,7 @@ def test_update(self):
None, None, None, None, None,
True))

# Test update with no composer.lock and quiet=True
# Test update with no vendor directory and quiet=True
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value=False)
Expand All @@ -110,7 +119,7 @@ def test_update(self):
None, None, None, None, None,
True))

# Test update with existing composer.lock
# Test update with existing vendor directory
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value=True)
Expand All @@ -120,7 +129,7 @@ def test_update(self):
with patch.dict(composer.__salt__, {'cmd.run_all': mock}):
self.assertEqual(composer.update('dir'), rval)

# Test update with no composer.lock
# Test update with no vendor directory
mock = MagicMock(return_value=True)
with patch.object(composer, '_valid_composer', mock):
mock = MagicMock(return_value=False)
Expand All @@ -132,7 +141,7 @@ def test_update(self):

def test_selfupdate(self):
'''
Test for Install composer dependencies for a directory.
Test for Composer selfupdate
'''
mock = MagicMock(return_value=False)
with patch.object(composer, '_valid_composer', mock):
Expand Down

0 comments on commit aef9701

Please sign in to comment.