[MediaWiki-commits] [Gerrit] operations...puppet-compiler[master]: Raise test coverage percentage
Giuseppe Lavagetto has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/363351 )
Change subject: Raise test coverage percentage
..
Raise test coverage percentage
Also, as a consequence:
* removed some dead code
* fixed a few errors here and there
Change-Id: Id04f2283df66f3742536367d814b9898c7a7
---
M puppet_compiler/controller.py
M puppet_compiler/prepare.py
A puppet_compiler/tests/fixtures/test_config.yaml.invalid
M puppet_compiler/tests/test_controller.py
M puppet_compiler/tests/test_directories.py
M puppet_compiler/tests/test_hostworker.py
M puppet_compiler/tests/test_prepare.py
7 files changed, 88 insertions(+), 20 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
Alexandros Kosiaris: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/puppet_compiler/controller.py b/puppet_compiler/controller.py
index 1d967a5..f2b61b3 100644
--- a/puppet_compiler/controller.py
+++ b/puppet_compiler/controller.py
@@ -45,7 +45,7 @@
try:
if configfile is not None:
self._parse_conf(configfile)
-except yaml.parser.ParserError as e:
+except yaml.error.YAMLError as e:
_log.exception("Configuration file %s contains malformed yaml: %s",
configfile, e)
sys.exit(2)
diff --git a/puppet_compiler/prepare.py b/puppet_compiler/prepare.py
index 670c6bc..c63dfd1 100644
--- a/puppet_compiler/prepare.py
+++ b/puppet_compiler/prepare.py
@@ -36,14 +36,6 @@
self.output_dir = FHS.output_dir
self.git = Git()
-def find_yaml(self, hostname):
-""" Finds facts file for the current hostname """
-facts_file = os.path.join(self.puppet_var, 'yaml', 'facts',
- '{}.yaml'.format(hostname))
-if os.path.isfile(facts_file):
-return facts_file
-return None
-
def cleanup(self):
"""
Remove the whole change tree.
@@ -216,14 +208,6 @@
def _pull_rebase_origin(self, origin_branch):
self.git.pull('--rebase', 'origin', origin_branch)
-
-def _sh(command):
-try:
-subprocess.check_call(command, shell=True)
-except subprocess.CalledProcessError as e:
-_log.error("Command '%s' failed with exit code '%s'",
- e.cmd, e.returncode)
-raise
class Git():
diff --git a/puppet_compiler/tests/fixtures/test_config.yaml.invalid
b/puppet_compiler/tests/fixtures/test_config.yaml.invalid
new file mode 100644
index 000..30e3d73
--- /dev/null
+++ b/puppet_compiler/tests/fixtures/test_config.yaml.invalid
@@ -0,0 +1,3 @@
+http_url: http://www.example.com/garbagehere
+test_non_existent:
+ - 'definitely': - 'maybe'
diff --git a/puppet_compiler/tests/test_controller.py
b/puppet_compiler/tests/test_controller.py
index 7066e84..c3291e1 100644
--- a/puppet_compiler/tests/test_controller.py
+++ b/puppet_compiler/tests/test_controller.py
@@ -24,6 +24,9 @@
self.assertEquals(len(c.config['test_non_existent']), 2)
self.assertEquals(c.config['http_url'],
'http://www.example.com/garbagehere')
+# This will log an error, but not raise an exception
+controller.Controller('unexistent', 19, 224570, 'test.eqiad.wmnet',
nthreads=2)
+self.assertRaises(SystemExit, controller.Controller, filename +
'.invalid', 1, 1, 'test.eqiad.wmnet')
@mock.patch('puppet_compiler.worker.HostWorker.html_index')
@mock.patch('puppet_compiler.worker.HostWorker.run_host')
@@ -39,6 +42,13 @@
c.m.prepare.assert_called_once_with()
c.m.refresh.assert_not_called()
self.assertEquals(c.state.modes['change']['fail'],
set(['test.eqiad.wmnet']))
+c.m.refresh.reset_mocks()
+c.config['puppet_src'] = '/src'
+c.config['puppet_private'] = '/private'
+mocker.return_value = (False, False, None)
+c.run()
+c.m.refresh.assert_has_calls([mock.call('/src'),
mock.call('/private')])
+self.assertEquals(c.state.modes['change']['noop'],
set(['test.eqiad.wmnet']))
def test_node_callback(self):
c = controller.Controller(None, 19, 224570, 'test.eqiad.wmnet')
@@ -60,6 +70,17 @@
c.on_node_compiled(response)
self.assertIn('test2.eqiad.wmnet', c.state.modes['change']['noop'])
self.assertEquals(c.count['change'], 7)
+with mock.patch('puppet_compiler.presentation.html.Index') as mocker:
+response = threads.Msg(
+is_error=False, value=(False, False, None), args=None,
+kwargs={
+'hostname': 'test2.eqiad.wmnet',
+'classes': (state.ChangeState, html.Index),
+'mode': 'change'}
+)
+c.count['change'] = 4
+
[MediaWiki-commits] [Gerrit] operations...puppet-compiler[master]: Raise test coverage percentage
Giuseppe Lavagetto has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/363351 )
Change subject: Raise test coverage percentage
..
Raise test coverage percentage
Also, as a consequence:
* removed some dead code
* fixed a few errors here and there
Change-Id: Id04f2283df66f3742536367d814b9898c7a7
---
M puppet_compiler/controller.py
M puppet_compiler/directories.py
M puppet_compiler/prepare.py
A puppet_compiler/tests/fixtures/test_config.yaml.invalid
M puppet_compiler/tests/test_controller.py
M puppet_compiler/tests/test_directories.py
M puppet_compiler/tests/test_hostworker.py
M puppet_compiler/tests/test_prepare.py
8 files changed, 83 insertions(+), 22 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/puppet-compiler
refs/changes/51/363351/1
diff --git a/puppet_compiler/controller.py b/puppet_compiler/controller.py
index 41b4641..fbc8311 100644
--- a/puppet_compiler/controller.py
+++ b/puppet_compiler/controller.py
@@ -45,7 +45,7 @@
try:
if configfile is not None:
self._parse_conf(configfile)
-except yaml.parser.ParserError as e:
+except yaml.error.YAMLError as e:
_log.exception("Configuration file %s contains malformed yaml: %s",
configfile, e)
sys.exit(2)
diff --git a/puppet_compiler/directories.py b/puppet_compiler/directories.py
index 214f776..bf4e41c 100644
--- a/puppet_compiler/directories.py
+++ b/puppet_compiler/directories.py
@@ -46,5 +46,5 @@
return os.path.join(base, 'catalogs', self.hostname + suffix + ext)
def outfile_for(self, env, what):
-name = os.basename(self.file_for(env, what))
-return os.path.join(self.output_dir, env + '.' + name)
+name = os.path.basename(self.file_for(env, what))
+return os.path.join(self.outdir, env + '.' + name)
diff --git a/puppet_compiler/prepare.py b/puppet_compiler/prepare.py
index 670c6bc..c63dfd1 100644
--- a/puppet_compiler/prepare.py
+++ b/puppet_compiler/prepare.py
@@ -36,14 +36,6 @@
self.output_dir = FHS.output_dir
self.git = Git()
-def find_yaml(self, hostname):
-""" Finds facts file for the current hostname """
-facts_file = os.path.join(self.puppet_var, 'yaml', 'facts',
- '{}.yaml'.format(hostname))
-if os.path.isfile(facts_file):
-return facts_file
-return None
-
def cleanup(self):
"""
Remove the whole change tree.
@@ -216,14 +208,6 @@
def _pull_rebase_origin(self, origin_branch):
self.git.pull('--rebase', 'origin', origin_branch)
-
-def _sh(command):
-try:
-subprocess.check_call(command, shell=True)
-except subprocess.CalledProcessError as e:
-_log.error("Command '%s' failed with exit code '%s'",
- e.cmd, e.returncode)
-raise
class Git():
diff --git a/puppet_compiler/tests/fixtures/test_config.yaml.invalid
b/puppet_compiler/tests/fixtures/test_config.yaml.invalid
new file mode 100644
index 000..30e3d73
--- /dev/null
+++ b/puppet_compiler/tests/fixtures/test_config.yaml.invalid
@@ -0,0 +1,3 @@
+http_url: http://www.example.com/garbagehere
+test_non_existent:
+ - 'definitely': - 'maybe'
diff --git a/puppet_compiler/tests/test_controller.py
b/puppet_compiler/tests/test_controller.py
index 77dcee6..f0815ed 100644
--- a/puppet_compiler/tests/test_controller.py
+++ b/puppet_compiler/tests/test_controller.py
@@ -23,6 +23,9 @@
self.assertEquals(len(c.config['test_non_existent']), 2)
self.assertEquals(c.config['http_url'],
'http://www.example.com/garbagehere')
+# This will log an error, but not raise an exception
+controller.Controller('unexistent', 19, 224570, 'test.eqiad.wmnet',
nthreads=2)
+self.assertRaises(SystemExit, controller.Controller, filename +
'.invalid', 1, 1, 'test.eqiad.wmnet')
@mock.patch('puppet_compiler.presentation.html.Index')
@mock.patch('puppet_compiler.worker.HostWorker.run_host')
@@ -38,6 +41,13 @@
c.m.prepare.assert_called_once_with()
c.m.refresh.assert_not_called()
self.assertEquals(c.state.modes['change']['fail'],
set(['test.eqiad.wmnet']))
+c.m.refresh.reset_mocks()
+c.config['puppet_src'] = '/src'
+c.config['puppet_private'] = '/private'
+mocker.return_value = (False, False, None)
+c.run()
+c.m.refresh.assert_has_calls([mock.call('/src'),
mock.call('/private')])
+self.assertEquals(c.state.modes['change']['noop'],
set(['test.eqiad.wmnet']))
def test_node_callback(self):
c = controller.Controller(None, 19, 224570, 'test.eqiad.wmnet')
@@ -59,6 +69,10 @@
c.on_node_compiled(response)
self.assertIn('test2.eqiad.wmnet',
