openstack/rally
Code Issues Proposed changes

Correction of tempest verification

Browse Source 
1) Revert direct usage of subunit

Test repository should be used as launcher for tempest tests, since direct
usage of `subunit` broke execution of `compute` set as well as `full` set.
Tests discovery and execution can't be split at the moment.

2) Cleanup of tempest folder afterfaulty installation

Also, execution of tempest sets cannot be correct without confidence in
correct installation of tempest repository. Cleanup of tempest folder after
faulty installation should be added to fix check of tempest installation.

3) Length of database field for verification results

MutableJSONEncodedDict limited by 64k. This length is not sufficient for
storage results of those verifications, which have a huge number of failures
and tracebacks.

4) Incorrect usage of mock in tests

Class `mock.MagicMock` doesn't have method `assert_called_once`.
Instead of this one, method `assert_called_once_with` should be used.
Based on the fact that this is critical error in testing, this patch
affects not only tempest verification.

Change-Id: I68660e38f2cae3f60a01e99cdbbb57bdb8a9bb04
Closes-Bug: #1313744
Closes-Bug: #1313742
Closes-Bug: #1305991
This commit is contained in:
Andrey Kurilin 2014-04-30 15:41:57 +03:00
parent 155c01cba9
commit 92a6960e3b
9 changed files with 63 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
rally/benchmark/validation.py
View File

@ -242,7 +242,7 @@ def tempest_tests_exists():
and test.split('.')[0] in consts.TEMPEST_TEST_SETS):
tests[tests.index(test)] = 'tempest.api.' + test
wrong_tests = set(tests) - set(allowed_tests)
wrong_tests = set(tests) - allowed_tests
if not wrong_tests:
return ValidationResult()

2
rally/db/sqlalchemy/models.py
View File

@ -215,7 +215,7 @@ class VerificationResult(BASE, RallyBase):
verification_uuid = sa.Column(sa.String(36),
sa.ForeignKey('verifications.uuid'))
data = sa.Column(sa_types.MutableJSONEncodedDict, nullable=False)
data = sa.Column(sa_types.BigMutableJSONEncodedDict, nullable=False)
def create_db():

62
rally/verification/verifiers/tempest/tempest.py
View File

@ -135,9 +135,11 @@ class Tempest(object):
self._install_venv()
self._initialize_testr()
except subprocess.CalledProcessError as e:
self.uninstall()
raise exceptions.TempestSetupFailure("failed cmd: '%s'", e.cmd)
else:
print("Tempest has been successfully installed!")
else:
print("Tempest is already installed")
@ -151,41 +153,42 @@ class Tempest(object):
self.generate_config_file()
if set_name == "full":
tests_arg = " ".join(self.discover_tests())
testr_arg = ""
elif set_name == "smoke":
tests_arg = " ".join(self.discover_tests("smoke"))
testr_arg = "smoke"
else:
tests_arg = " ".join(self.discover_tests("tempest.api.%s" %
set_name))
if regex:
tests_arg += " %s" % " ".join(self.discover_tests(regex))
if set_name:
testr_arg = "tempest.api.%s" % set_name
elif regex:
testr_arg = regex
else:
testr_arg = ""
self.verification.start_verifying(set_name)
try:
self.run(tests_arg)
self.run(testr_arg)
except subprocess.CalledProcessError:
print("Test set %s has been finished with error. "
"Check log for details" % set_name)
def run(self, test_arg):
def run(self, testr_arg=None):
"""Launch tempest with given arguments
:param test_arg: argument which will be transmitted into test launcher
:type test_arg: str
:param testr_arg: argument which will be transmitted into testr
:type testr_arg: str
:raises: :class:`subprocess.CalledProcessError` if tests has been
finished with error.
"""
test_cmd = (
"%(venv)s python -m subunit.run %(arg)s "
"%(venv)s testr run --parallel --subunit %(arg)s "
"| %(venv)s subunit2junitxml --forward --output-to=%(log_file)s "
"| %(venv)s subunit-2to1 "
"| %(venv)s %(tempest_path)s/tools/colorizer.py" %
{
"venv": self.venv_wrapper,
"arg": test_arg,
"arg": testr_arg,
"tempest_path": self.tempest_path,
"log_file": self.log_file
})
@ -194,7 +197,7 @@ class Tempest(object):
env=self.env, shell=True)
def discover_tests(self, pattern=""):
"""Return a list with discovered tests which match given pattern."""
"""Return a set of discovered tests which match given pattern."""
cmd = "%(venv)s testr list-tests %(pattern)s" % {
"venv": self.venv_wrapper,
@ -203,21 +206,23 @@ class Tempest(object):
cmd, shell=True, cwd=self.tempest_path, env=self.env,
stdout=subprocess.PIPE).communicate()[0]
tests = []
tests = set()
for test in raw_results.split('\n'):
if test.startswith("tempest."):
index = test.find("[")
if index != -1:
tests.append(test[:index])
tests.add(test[:index])
else:
tests.append(test)
tests.add(test)
return tests
@utils.log_verification_wrapper(
LOG.info, _("Saving verification results."))
def _save_results(self):
if os.path.isfile(self.log_file):
dom = md.parse(self.log_file).getElementsByTagName("testsuite")[0]
@staticmethod
def parse_results(log_file):
"""Parse junitxml file."""
if os.path.isfile(log_file):
dom = md.parse(log_file).getElementsByTagName("testsuite")[0]
total = {
"tests": int(dom.getAttribute("tests")),
@ -246,11 +251,18 @@ class Tempest(object):
else:
test["status"] = "OK"
test_cases[test["name"]] = test
if self.verification:
self.verification.finish_verification(total=total,
test_cases=test_cases)
return total, test_cases
else:
LOG.error("XML-log file not found.")
return None, None
@utils.log_verification_wrapper(
LOG.info, _("Saving verification results."))
def _save_results(self):
total, test_cases = self.parse_results(self.log_file)
if total and test_cases and self.verification:
self.verification.finish_verification(total=total,
test_cases=test_cases)
def verify(self, set_name, regex):
self._prepare_and_run(set_name, regex)

2
tests/benchmark/scenarios/ceilometer/test_ceilometer.py
View File

@ -36,7 +36,7 @@ class CeilometerBasicTestCase(test.TestCase):
scenario._list_alarms = mock.MagicMock()
scenario.list_alarms()
scenario._list_alarms.assert_called_once()
scenario._list_alarms.assert_called_once_with()
def test_create_and_list_alarm(self):
fake_alarm = mock.MagicMock()

4
tests/benchmark/scenarios/keystone/test_utils.py
View File

@ -130,7 +130,7 @@ class KeystoneScenarioTestCase(test.TestCase):
fake_clients._keystone = fake_keystone
scenario = utils.KeystoneScenario(admin_clients=fake_clients)
scenario._list_users()
fake_keystone.users.list.assert_called_once()
fake_keystone.users.list.assert_called_once_with()
self._test_atomic_action_timer(scenario.atomic_actions(),
'keystone.list_users')
@ -141,6 +141,6 @@ class KeystoneScenarioTestCase(test.TestCase):
fake_clients._keystone = fake_keystone
scenario = utils.KeystoneScenario(admin_clients=fake_clients)
scenario._list_tenants()
fake_keystone.tenants.list.assert_called_once()
fake_keystone.tenants.list.assert_called_once_with()
self._test_atomic_action_timer(scenario.atomic_actions(),
'keystone.list_tenants')

2
tests/benchmark/scenarios/tempest/test_tempest.py
View File

@ -35,7 +35,7 @@ class TempestScenarioTestCase(test.TestCase):
def test_single_test(self, mock_sp):
self.scenario.single_test("tempest.api.fake.test")
expected_call = (
"%(venv)s python -m subunit.run tempest.api.fake.test "
"%(venv)s testr run --parallel --subunit tempest.api.fake.test "
"| %(venv)s subunit2junitxml --forward --output-to=/dev/null "
"| %(venv)s subunit-2to1 "
"| %(venv)s %(tempest_path)s/tools/colorizer.py" %

4
tests/deploy/test_lxc.py
View File

@ -73,7 +73,7 @@ class LxcEngineTestCase(test.TestCase):
mock.call.get_server_object('name'),
mock.call.stop_containers()]
self.assertEqual(host_calls, fake_host.mock_calls)
fake_engine.deploy.assert_called_once()
fake_engine.deploy.assert_called_once_with()
m_engine.EngineFactory.get_engine.assert_called_once_with(
'FakeEngine', fake_deployment)
engine_config = self.config['engine'].copy()
@ -190,4 +190,4 @@ class LxcEngineTestCase(test.TestCase):
for res in fake_resources:
m_deployment.assert_has_calls(mock.call.delete_resource(res.id))
fake_provider.destroy_servers.assert_called_once()
fake_provider.destroy_servers.assert_called_once_with()

4
tests/deploy/test_multihost.py
View File

@ -74,8 +74,8 @@ class TestMultihostEngine(test.TestCase):
fakeDeployment.assert_called_once_with(
config=self.config['nodes'][0],
parent_uuid=self.deployment['uuid'])
fake_engine.__enter__.assert_called_once()
fake_engine.__exit__.assert_called_once()
fake_engine.__enter__.assert_called_once_with()
fake_engine.__exit__.assert_called_once_with(None, None, None)
def test__update_controller_ip(self):
self.engine.controller_ip = '1.2.3.4'

53
tests/verification/verifiers/test_tempest.py
View File

@ -13,8 +13,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import json
import mock
import os
@ -44,8 +42,8 @@ class TempestTestCase(test.TestCase):
mock_open.return_value = mock_file
self.verifier._write_config(conf)
mock_open.assert_called_once_with(self.verifier.config_file, 'w+')
mock_file.write.assert_called_once_whith(conf)
mock_file.close.assert_called_once()
conf.write.assert_called_once_with(mock_file.__enter__())
mock_file.__exit__.assert_called_once_with(None, None, None)
@mock.patch('os.path.exists')
def test_is_installed(self, mock_exists):
@ -95,7 +93,7 @@ class TempestTestCase(test.TestCase):
def test_run(self, mock_sp, mock_env):
self.verifier.run('tempest.api.image')
fake_call = (
'%(venv)s python -m subunit.run tempest.api.image '
'%(venv)s testr run --parallel --subunit tempest.api.image '
'| %(venv)s subunit2junitxml --forward '
'--output-to=%(tempest_path)s/tests_log.xml '
'| %(venv)s subunit-2to1 '
@ -106,44 +104,25 @@ class TempestTestCase(test.TestCase):
fake_call, env=mock_env, cwd=self.verifier.tempest_path,
shell=True)
@mock.patch(TEMPEST_PATH + '.tempest.os.remove')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest.discover_tests')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest._initialize_testr')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest.run')
@mock.patch(TEMPEST_PATH + '.tempest.Tempest._write_config')
@mock.patch(TEMPEST_PATH + '.config.TempestConf.generate')
@mock.patch(TEMPEST_PATH + '.config.TempestConf')
@mock.patch('rally.db.deployment_get')
@mock.patch('rally.osclients.Clients')
@mock.patch('rally.objects.endpoint.Endpoint')
@mock.patch(TEMPEST_PATH + '.config.urllib2')
def test_verify(self, mock_urllib2, mock_endpoint, mock_osclients,
mock_get, mock_gen, mock_write, mock_run, mock_testr_init,
mock_discover):
def test_verify(self, mock_endpoint, mock_osclients, mock_get, mock_conf,
mock_write, mock_run, mock_testr_init, mock_discover,
mock_os):
fake_conf = mock.MagicMock()
mock_gen.return_value = fake_conf
class MockResponse(object):
def __init__(self, resp_data, code=200, msg='OK'):
self.resp_data = resp_data
self.code = code
self.msg = msg
self.headers = {'content-type': 'text/plain; charset=utf-8'}
def read(self):
return self.resp_data
def getcode(self):
return self.code
mock_urllib2.urlopen.return_value = MockResponse(json.dumps({}))
fake_tests = ['tempest.test.fake1', 'tempest.test.fake2']
mock_discover.return_value = fake_tests
mock_conf().generate.return_value = fake_conf
self.verifier.verify("smoke", None)
mock_gen.assert_called_once()
mock_conf().generate.assert_called_once_with()
mock_write.assert_called_once_with(fake_conf)
mock_run.assert_called_once_with(' '.join(fake_tests))
mock_run.assert_called_once_with("smoke")
@mock.patch('os.environ')
def test__generate_env(self, mock_env):
@ -166,7 +145,7 @@ class TempestTestCase(test.TestCase):
mock_isdir.assert_called_once_with(
os.path.join(self.verifier.tempest_path, '.venv'))
self.assertEqual(0, mock_sp.call_count)
self.assertFalse(mock_sp.called)
@mock.patch('os.path.isdir')
@mock.patch(TEMPEST_PATH + '.tempest.subprocess.check_call')
@ -194,7 +173,7 @@ class TempestTestCase(test.TestCase):
mock_isdir.assert_called_once_with(
os.path.join(self.verifier.tempest_path, '.testrepository'))
self.assertEqual(0, mock_sp.call_count)
self.assertFalse(mock_sp.called)
@mock.patch('os.path.isdir')
@mock.patch(TEMPEST_PATH + '.tempest.subprocess.check_call')
@ -219,12 +198,12 @@ class TempestTestCase(test.TestCase):
mock_isfile.assert_called_once_with(self.verifier.log_file)
self.assertEqual(0, mock_parse.call_count)
@mock.patch('xml.dom.minidom.parse')
@mock.patch('os.path.isfile')
def test__save_results_with_log_file(self, mock_isfile, mock_parse):
def test__save_results_with_log_file(self, mock_isfile):
mock_isfile.return_value = True
self.verifier.log_file = os.path.join(os.path.dirname(__file__),
'fake_log.xml')
self.verifier._save_results()
mock_isfile.assert_called_once_with(self.verifier.log_file)
self.verifier.verification.finish_verification.assert_called_once()
self.assertEqual(
1, self.verifier.verification.finish_verification.call_count)