Skip to content
This repository has been archived by the owner on Sep 16, 2020. It is now read-only.

Commit

Permalink
Merge pull request #103 from AlanCoding/fixes231
Browse files Browse the repository at this point in the history
Bug fixes revealed in pre-release testing
  • Loading branch information
AlanCoding committed Dec 10, 2015
2 parents d536be5 + ddee1a9 commit 1801771
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 50 deletions.
9 changes: 7 additions & 2 deletions lib/tower_cli/resources/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def launch(self, job_template=None, tags=None, monitor=False, timeout=None,

# Add the runtime extra_vars to this list
if extra_vars:
extra_vars_list += extra_vars
extra_vars_list += list(extra_vars) # accept tuples

# If the job template requires prompting for extra variables,
# do so (unless --no-input is set).
Expand All @@ -109,7 +109,12 @@ def launch(self, job_template=None, tags=None, monitor=False, timeout=None,
initial,
))
extra_vars = click.edit(initial) or ''
extra_vars_list = [extra_vars]
if extra_vars != initial:
extra_vars_list = [extra_vars]

# Data is starting out with JT variables, and we only want to
# include extra_vars that come from the algorithm here.
data.pop('extra_vars', None)

# Dump extra_vars into JSON string for launching job
if len(extra_vars_list) > 0:
Expand Down
2 changes: 1 addition & 1 deletion lib/tower_cli/resources/job_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def create(self, fail_on_found=False, force_on_exists=False,
extra_vars, force_json=False
)
# Provide a default value for job_type, but only in creation of JT
if 'job_type' not in kwargs:
if not kwargs.get('job_type', False):
kwargs['job_type'] = 'run'
return super(Resource, self).create(
fail_on_found=fail_on_found, force_on_exists=force_on_exists,
Expand Down
108 changes: 61 additions & 47 deletions tests/test_resources_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,26 @@ def standard_registration(t):
t.register_json('/job_templates/1/launch/', {'job': 42}, method='POST')


def jt_vars_registration(t, extra_vars):
""" Endpoints that are needed to get information from job template.
This particular combination also entails
1) version of Tower - 2.2.0
2) sucessful job launch, id=42
3) prompts user for variables on launch """
t.register_json('/job_templates/1/', {
'ask_variables_on_launch': True,
'extra_vars': extra_vars,
'id': 1,
'name': 'frobnicate',
'related': {'launch': '/job_templates/1/launch/'},
})
register_get(t)
t.register_json('/config/', {'version': '2.2.0'}, method='GET')
t.register_json('/job_templates/1/launch/', {}, method='GET')
t.register_json('/job_templates/1/launch/', {'job': 42},
method='POST')


class LaunchTests(unittest.TestCase):
"""A set of tests for ensuring that the job resource's launch command
works in the way we expect.
Expand Down Expand Up @@ -101,6 +121,15 @@ def test_launch_w_tags(self):
json.loads(t.requests[2].body)['job_tags'], 'a, b, c',
)

def test_launch_w_tuple_extra_vars(self):
"""Establish that if the click library gives a tuple, than the job
will run normally.
"""
with client.test_mode as t:
standard_registration(t)
result = self.res.launch(1, extra_vars=())
self.assertDictContainsSubset({'changed': True, 'id': 42}, result)

def test_basic_launch_monitor_option(self):
"""Establish that we are able to create a job that doesn't require
any invocation-time input, and that monitor is called if requested.
Expand All @@ -117,18 +146,7 @@ def test_extra_vars_at_runtime(self):
"""
with client.test_mode as t:
# test with JSON job template extra_vars
t.register_json('/job_templates/1/', {
'ask_variables_on_launch': True,
'extra_vars': '{"spam": "eggs"}',
'id': 1,
'name': 'frobnicate',
'related': {'launch': '/job_templates/1/launch/'},
})
register_get(t)
t.register_json('/config/', {'version': '2.2.0'}, method='GET')
t.register_json('/job_templates/1/launch/', {}, method='GET')
t.register_json('/job_templates/1/launch/', {'job': 42},
method='POST')
jt_vars_registration(t, '{"spam": "eggs"}')
with mock.patch.object(click, 'edit') as edit:
edit.return_value = '# Nothing.\nfoo: bar'
result = self.res.launch(1, no_input=False)
Expand All @@ -142,64 +160,60 @@ def test_extra_vars_at_runtime(self):
)
self.assertDictContainsSubset({'changed': True, 'id': 42}, result)

def test_extra_vars_at_runtime_YAML_JT(self):
"""Establish that if we should be asking for extra variables at
runtime, that we do.
"""
with client.test_mode as t:
# test with YAML and comments
t.register_json('/job_templates/1/', {
'ask_variables_on_launch': True,
'extra_vars': 'spam: eggs\n# comment',
'id': 1,
'name': 'frobnicate',
'related': {'launch': '/job_templates/1/launch/'},
})
jt_vars_registration(t, 'spam: eggs\n# comment')
with mock.patch.object(click, 'edit') as edit:
edit.return_value = '# Nothing.\nfoo: bar'
result = self.res.launch(
1, no_input=False
)
self.res.launch(1, no_input=False)
self.assertIn('# comment', edit.mock_calls[0][1][0])
self.assertDictContainsSubset(
{"spam": "eggs"},
yaml.load(edit.mock_calls[0][1][0])
)

def test_extra_vars_at_runtime_no_user_data(self):
"""User launches a job that prompts for variables. User closes
editor without adding any text.
Establish that we launch the job as-is.
"""
with client.test_mode as t:
# No job template variables
jt_vars_registration(t, '')
initial = '\n'.join((
'# Specify extra variables (if any) here as YAML.',
'# Lines beginning with "#" denote comments.',
'',
))
with mock.patch.object(click, 'edit') as edit:
edit.return_value = initial
self.res.launch(1, no_input=False)
self.assertEqual(t.requests[2].method, 'POST')
self.assertEqual(t.requests[2].body, '{}')

def test_job_template_variables(self):
"""Establish that job template extra_vars are combined with local
extra vars, but only for older versions
"""
with client.test_mode as t:
t.register_json('/job_templates/1/', {
'ask_variables_on_launch': True,
'extra_vars': 'spam: eggs',
'id': 1,
'name': 'frobnicate',
'related': {'launch': '/job_templates/1/launch/'},
})
register_get(t)
t.register_json('/config/', {'version': '2.2.0'}, method='GET')
t.register_json('/job_templates/1/launch/', {}, method='GET')
t.register_json('/job_templates/1/launch/', {'job': 42},
method='POST')
jt_vars_registration(t, 'spam: eggs')
result = self.res.launch(1, extra_vars=['foo: bar'])
response_json = yaml.load(t.requests[3].body)
ev_json = yaml.load(response_json['extra_vars'])
self.assertTrue('foo' in ev_json)
self.assertTrue('spam' in ev_json)
self.assertDictContainsSubset({'changed': True, 'id': 42}, result)

# check that in recent versions, it does not include job template
# variables along with the rest
def test_job_template_variables_post_24(self):
""" Check that in recent versions, it does not include job template
variables along with the rest """
with client.test_mode as t:
t.register_json('/job_templates/1/', {
'ask_variables_on_launch': True,
'extra_vars': 'spam: eggs',
'id': 1,
'name': 'frobnicate',
'related': {'launch': '/job_templates/1/launch/'},
})
register_get(t)
jt_vars_registration(t, 'spam: eggs')
t.register_json('/config/', {'version': '2.4'}, method='GET')
t.register_json('/job_templates/1/launch/', {}, method='GET')
t.register_json('/job_templates/1/launch/', {'job': 42},
method='POST')
result = self.res.launch(1, extra_vars=['foo: bar'])
response_json = yaml.load(t.requests[3].body)
ev_json = yaml.load(response_json['extra_vars'])
Expand Down

0 comments on commit 1801771

Please sign in to comment.