Add the ability for users to add test attributes. Non-user-created attributes (added by the parser) are still immutable.
* add boolean column user_created to TestAttribute to keep track of which are user created, so we can preserve immutability
* add id primary key field to test attributes. Django requires this and we've been squeezing by without it, but we can't anymore.
* declare some PK fields AutoFields in TKO models, as they should be. this didn't matter before but does now that we have a TKO unit test, since these models determine how the test DB gets created.
* add set_test_attribute RPC to set/delete attributes
* modify get_detailed_test_views() to use the new populate_relationships() method to gather attributes and labels much more efficiently
* add rpc_interface_unittest, a unit test for the TKO rpc interface. TKO was previously completely untested, so this is the first unit test of any kind for it. since the doctests on AFE turned out to be quite unpopular, I'm using the unittest framework this time. this required some changes to AFE testing code.
* various fixes to model_logic to account for assumptions we were previously making that aren't true in TKO (mostly about PK fields being named "id").
Note that the migration may be slow, as it's adding two columns to test_attributes, a potentially large table.
Signed-off-by: Steve Howard <showard@google.com>
git-svn-id: http://test.kernel.org/svn/autotest/trunk@3109 592f7852-d20e-0410-864c-8624ca9c26a4
diff --git a/frontend/afe/model_logic.py b/frontend/afe/model_logic.py
index 35a8b50..5610e7a 100644
--- a/frontend/afe/model_logic.py
+++ b/frontend/afe/model_logic.py
@@ -308,7 +308,7 @@
if not model_objects:
# if we don't bail early, we'll get a SQL error later
return
- id_list = (item.id for item in model_objects)
+ id_list = (item._get_pk_val() for item in model_objects)
pivot_table, pivot_from_field, pivot_to_field = (
self._determine_pivot_table(related_model))
related_ids = self._query_pivot_table(id_list, pivot_table,
@@ -318,7 +318,7 @@
related_objects_by_id = related_model.objects.in_bulk(all_related_ids)
for item in model_objects:
- related_ids_for_item = related_ids.get(item.id, [])
+ related_ids_for_item = related_ids.get(item._get_pk_val(), [])
related_objects = [related_objects_by_id[related_id]
for related_id in related_ids_for_item]
setattr(item, related_list_name, related_objects)
@@ -374,7 +374,7 @@
continue
value = data[field.name]
if isinstance(value, dbmodels.Model):
- data[field.name] = value.id
+ data[field.name] = value._get_pk_val()
@classmethod
@@ -385,7 +385,7 @@
a problem, but it can be annoying in certain situations.
"""
for field in cls._meta.fields:
- if type(field) == dbmodels.BooleanField:
+ if type(field) == dbmodels.BooleanField and field.name in data:
data[field.name] = bool(data[field.name])
@@ -453,8 +453,10 @@
elif field_obj.rel:
dest_obj = field_obj.rel.to.smart_get(data[field_name],
valid_only=False)
- if to_human_readable and dest_obj.name_field is not None:
- data[field_name] = getattr(dest_obj, dest_obj.name_field)
+ if to_human_readable:
+ if dest_obj.name_field is not None:
+ data[field_name] = getattr(dest_obj,
+ dest_obj.name_field)
else:
data[field_name] = dest_obj
@@ -780,3 +782,43 @@
def _prepare(cls, model):
super(ModelWithInvalid.Manipulator, cls)._prepare(model)
cls.manager = model.valid_objects
+
+
+class ModelWithAttributes(object):
+ """
+ Mixin class for models that have an attribute model associated with them.
+ The attribute model is assumed to have its value field named "value".
+ """
+
+ def _get_attribute_model_and_args(self, attribute):
+ """
+ Subclasses should override this to return a tuple (attribute_model,
+ keyword_args), where attribute_model is a model class and keyword_args
+ is a dict of args to pass to attribute_model.objects.get() to get an
+ instance of the given attribute on this object.
+ """
+ raise NotImplemented
+
+
+ def set_attribute(self, attribute, value):
+ attribute_model, get_args = self._get_attribute_model_and_args(
+ attribute)
+ attribute_object, _ = attribute_model.objects.get_or_create(**get_args)
+ attribute_object.value = value
+ attribute_object.save()
+
+
+ def delete_attribute(self, attribute):
+ attribute_model, get_args = self._get_attribute_model_and_args(
+ attribute)
+ try:
+ attribute_model.objects.get(**get_args).delete()
+ except HostAttribute.DoesNotExist:
+ pass
+
+
+ def set_or_delete_attribute(self, attribute, value):
+ if value is None:
+ self.delete_attribute(attribute)
+ else:
+ self.set_attribute(attribute, value)
diff --git a/frontend/afe/models.py b/frontend/afe/models.py
index 053680d..a4373f8 100644
--- a/frontend/afe/models.py
+++ b/frontend/afe/models.py
@@ -175,7 +175,8 @@
return self.login
-class Host(model_logic.ModelWithInvalid, dbmodels.Model):
+class Host(model_logic.ModelWithInvalid, dbmodels.Model,
+ model_logic.ModelWithAttributes):
"""\
Required:
hostname
@@ -322,18 +323,8 @@
return active[0]
- def set_attribute(self, attribute, value):
- attribute_object = HostAttribute.objects.get_or_create(
- host=self, attribute=attribute)[0]
- attribute_object.value = value
- attribute_object.save()
-
-
- def delete_attribute(self, attribute):
- try:
- HostAttribute.objects.get(host=self, attribute=attribute).delete()
- except HostAttribute.DoesNotExist:
- pass
+ def _get_attribute_model_and_args(self, attribute):
+ return HostAttribute, dict(host=self, attribute=attribute)
class Meta:
diff --git a/frontend/afe/rpc_interface.py b/frontend/afe/rpc_interface.py
index c95f231..36cea00 100644
--- a/frontend/afe/rpc_interface.py
+++ b/frontend/afe/rpc_interface.py
@@ -134,10 +134,7 @@
models.AclGroup.check_for_acl_violation_hosts(hosts)
for host in hosts:
- if value is None:
- host.delete_attribute(attribute)
- else:
- host.set_attribute(attribute, value)
+ host.set_or_delete_attribute(attribute, value)
def delete_host(id):
diff --git a/frontend/frontend_unittest.py b/frontend/frontend_unittest.py
index 6aac47d..15f4edc 100644
--- a/frontend/frontend_unittest.py
+++ b/frontend/frontend_unittest.py
@@ -10,11 +10,11 @@
class FrontendTest(unittest.TestCase):
def setUp(self):
- readonly_connection.ReadOnlyConnection.set_testing_mode(True)
+ setup_test_environment.set_up()
def tearDown(self):
- readonly_connection.ReadOnlyConnection.set_testing_mode(False)
+ setup_test_environment.tear_down()
def test_all(self):
diff --git a/frontend/setup_test_environment.py b/frontend/setup_test_environment.py
index ef726d1..84df024 100644
--- a/frontend/setup_test_environment.py
+++ b/frontend/setup_test_environment.py
@@ -12,10 +12,11 @@
settings.DATABASE_NAME = ':memory:'
from django.db import connection
+from autotest_lib.frontend.afe import readonly_connection
def set_test_database(database):
settings.DATABASE_NAME = database
- connection.close()
+ destroy_test_database()
def backup_test_database():
@@ -36,3 +37,26 @@
def run_syncdb(verbosity=0):
management.syncdb(verbosity, interactive=False)
+
+
+def destroy_test_database():
+ connection.close()
+ # Django brilliantly ignores close() requests on in-memory DBs to keep us
+ # naive users from accidentally destroying data. So reach in and close
+ # the real connection ourselves.
+ # Note this depends on Django internals and will likely need to be changed
+ # when we move to Django 1.x.
+ real_connection = connection.connection
+ if real_connection is not None:
+ real_connection.close()
+ connection.connection = None
+
+
+def set_up():
+ run_syncdb()
+ readonly_connection.ReadOnlyConnection.set_testing_mode(True)
+
+
+def tear_down():
+ readonly_connection.ReadOnlyConnection.set_testing_mode(False)
+ destroy_test_database()
diff --git a/new_tko/tko/models.py b/new_tko/tko/models.py
index cbbdfdf..dd02fa8 100644
--- a/new_tko/tko/models.py
+++ b/new_tko/tko/models.py
@@ -99,7 +99,7 @@
class Machine(dbmodels.Model):
- machine_idx = dbmodels.IntegerField(primary_key=True)
+ machine_idx = dbmodels.AutoField(primary_key=True)
hostname = dbmodels.CharField(unique=True, maxlength=300)
machine_group = dbmodels.CharField(blank=True, maxlength=240)
owner = dbmodels.CharField(blank=True, maxlength=240)
@@ -109,7 +109,7 @@
class Kernel(dbmodels.Model):
- kernel_idx = dbmodels.IntegerField(primary_key=True)
+ kernel_idx = dbmodels.AutoField(primary_key=True)
kernel_hash = dbmodels.CharField(maxlength=105, editable=False)
base = dbmodels.CharField(maxlength=90)
printable = dbmodels.CharField(maxlength=300)
@@ -129,7 +129,7 @@
class Status(dbmodels.Model):
- status_idx = dbmodels.IntegerField(primary_key=True)
+ status_idx = dbmodels.AutoField(primary_key=True)
word = dbmodels.CharField(maxlength=30)
class Meta:
@@ -137,7 +137,7 @@
class Job(dbmodels.Model):
- job_idx = dbmodels.IntegerField(primary_key=True)
+ job_idx = dbmodels.AutoField(primary_key=True)
tag = dbmodels.CharField(unique=True, maxlength=300)
label = dbmodels.CharField(maxlength=300)
username = dbmodels.CharField(maxlength=240)
@@ -150,8 +150,9 @@
db_table = 'jobs'
-class Test(dbmodels.Model, model_logic.ModelExtensions):
- test_idx = dbmodels.IntegerField(primary_key=True)
+class Test(dbmodels.Model, model_logic.ModelExtensions,
+ model_logic.ModelWithAttributes):
+ test_idx = dbmodels.AutoField(primary_key=True)
job = dbmodels.ForeignKey(Job, db_column='job_idx')
test = dbmodels.CharField(maxlength=90)
subdir = dbmodels.CharField(blank=True, maxlength=180)
@@ -162,40 +163,64 @@
finished_time = dbmodels.DateTimeField(null=True, blank=True)
started_time = dbmodels.DateTimeField(null=True, blank=True)
+ objects = model_logic.ExtendedManager()
+
+ def _get_attribute_model_and_args(self, attribute):
+ return TestAttribute, dict(test=self, attribute=attribute,
+ user_created=True)
+
+
+ def set_attribute(self, attribute, value):
+ # ensure non-user-created attributes remain immutable
+ try:
+ TestAttribute.objects.get(test=self, attribute=attribute,
+ user_created=False)
+ raise ValueError('Attribute %s already exists for test %s and is '
+ 'immutable' % (attribute, self.test_idx))
+ except TestAttribute.DoesNotExist:
+ super(Test, self).set_attribute(attribute, value)
+
+
class Meta:
db_table = 'tests'
class TestAttribute(dbmodels.Model, model_logic.ModelExtensions):
- # this isn't really a primary key, but it's necessary to appease Django
- # and is harmless as long as we're careful
- test = dbmodels.ForeignKey(Test, db_column='test_idx', primary_key=True)
+ test = dbmodels.ForeignKey(Test, db_column='test_idx')
attribute = dbmodels.CharField(maxlength=90)
value = dbmodels.CharField(blank=True, maxlength=300)
+ user_created = dbmodels.BooleanField(default=False)
+
+ objects = model_logic.ExtendedManager()
class Meta:
db_table = 'test_attributes'
class IterationAttribute(dbmodels.Model, model_logic.ModelExtensions):
- # see comment on TestAttribute regarding primary_key=True
+ # this isn't really a primary key, but it's necessary to appease Django
+ # and is harmless as long as we're careful
test = dbmodels.ForeignKey(Test, db_column='test_idx', primary_key=True)
iteration = dbmodels.IntegerField()
attribute = dbmodels.CharField(maxlength=90)
value = dbmodels.CharField(blank=True, maxlength=300)
+ objects = model_logic.ExtendedManager()
+
class Meta:
db_table = 'iteration_attributes'
class IterationResult(dbmodels.Model, model_logic.ModelExtensions):
- # see comment on TestAttribute regarding primary_key=True
+ # see comment on IterationAttribute regarding primary_key=True
test = dbmodels.ForeignKey(Test, db_column='test_idx', primary_key=True)
iteration = dbmodels.IntegerField()
attribute = dbmodels.CharField(maxlength=90)
value = dbmodels.FloatField(null=True, max_digits=12, decimal_places=31,
blank=True)
+ objects = model_logic.ExtendedManager()
+
class Meta:
db_table = 'iteration_result'
@@ -207,6 +232,7 @@
filter_interface=dbmodels.HORIZONTAL)
name_field = 'name'
+ objects = model_logic.ExtendedManager()
class Meta:
db_table = 'test_labels'
diff --git a/new_tko/tko/rpc_interface.py b/new_tko/tko/rpc_interface.py
index 5070c44..7987e49 100644
--- a/new_tko/tko/rpc_interface.py
+++ b/new_tko/tko/rpc_interface.py
@@ -158,19 +158,13 @@
# test detail view
-def _itermodel_to_list(test_id, iteration_model):
- return iteration_model.list_objects(
- dict(test__test_idx=test_id),
- fields=('iteration', 'attribute', 'value'))
-
-
def _attributes_to_dict(attribute_list):
- return dict((attribute_dict['attribute'], attribute_dict['value'])
- for attribute_dict in attribute_list)
+ return dict((attribute.attribute, attribute.value)
+ for attribute in attribute_list)
def _iteration_attributes_to_dict(attribute_list):
- iter_keyfunc = operator.itemgetter('iteration')
+ iter_keyfunc = operator.attrgetter('iteration')
attribute_list.sort(key=iter_keyfunc)
iterations = {}
for key, group in itertools.groupby(attribute_list, iter_keyfunc):
@@ -178,38 +172,37 @@
return iterations
+def _format_iteration_keyvals(test):
+ iteration_attr = _iteration_attributes_to_dict(test.iteration_attributes)
+ iteration_perf = _iteration_attributes_to_dict(test.iteration_results)
+
+ all_iterations = iteration_attr.keys() + iteration_perf.keys()
+ max_iterations = max(all_iterations + [0])
+
+ # merge the iterations into a single list of attr & perf dicts
+ return [{'attr': iteration_attr.get(index, {}),
+ 'perf': iteration_perf.get(index, {})}
+ for index in xrange(1, max_iterations + 1)]
+
+
def get_detailed_test_views(**filter_data):
test_views = models.TestView.list_objects(filter_data)
+ tests_by_id = models.Test.objects.in_bulk([test_view['test_idx']
+ for test_view in test_views])
+ tests = tests_by_id.values()
+ models.Test.objects.populate_relationships(tests, models.TestAttribute,
+ 'attributes')
+ models.Test.objects.populate_relationships(tests, models.IterationAttribute,
+ 'iteration_attributes')
+ models.Test.objects.populate_relationships(tests, models.IterationResult,
+ 'iteration_results')
+ models.Test.objects.populate_relationships(tests, models.TestLabel,
+ 'labels')
for test_view in test_views:
- test_id = test_view['test_idx']
-
- # load in the test keyvals
- attribute_dicts = models.TestAttribute.list_objects(
- dict(test__test_idx=test_id), fields=('attribute', 'value'))
- test_view['attributes'] = _attributes_to_dict(attribute_dicts)
-
- # load in the iteration keyvals
- attr_dicts = _itermodel_to_list(test_id, models.IterationAttribute)
- perf_dicts = _itermodel_to_list(test_id, models.IterationResult)
-
- # convert the iterations into dictionarys and count total iterations
- iteration_attr = _iteration_attributes_to_dict(attr_dicts)
- iteration_perf = _iteration_attributes_to_dict(perf_dicts)
- all_dicts = attr_dicts + perf_dicts
- if all_dicts:
- max_iterations = max(row['iteration'] for row in all_dicts)
- else:
- max_iterations = 0
-
- # merge the iterations into a single list of attr & perf dicts
- test_view['iterations'] = [{'attr': iteration_attr.get(index, {}),
- 'perf': iteration_perf.get(index, {})}
- for index in xrange(1, max_iterations + 1)]
-
- # load in the test labels
- label_dicts = models.TestLabel.list_objects(
- dict(tests__test_idx=test_id), fields=('name',))
- test_view['labels'] = [label_dict['name'] for label_dict in label_dicts]
+ test = tests_by_id[test_view['test_idx']]
+ test_view['attributes'] = _attributes_to_dict(test.attributes)
+ test_view['iterations'] = _format_iteration_keyvals(test)
+ test_view['labels'] = [label.name for label in test.labels]
return rpc_utils.prepare_for_serialization(test_views)
# graphing view support
@@ -329,6 +322,23 @@
label.tests.remove(*test_ids)
+# user-created test attributes
+
+def set_test_attribute(attribute, value, **test_filter_data):
+ """
+ * attribute - string name of attribute
+ * value - string, or None to delete an attribute
+ * test_filter_data - filter data to apply to TestView to choose tests to act
+ upon
+ """
+ assert test_filter_data # disallow accidental actions on all hosts
+ test_ids = models.TestView.objects.query_test_ids(test_filter_data)
+ tests = models.Test.objects.in_bulk(test_ids)
+
+ for test in tests.itervalues():
+ test.set_or_delete_attribute(attribute, value)
+
+
# saved queries
def get_saved_queries(**filter_data):
diff --git a/new_tko/tko/rpc_interface_unittest.py b/new_tko/tko/rpc_interface_unittest.py
new file mode 100644
index 0000000..dedf8cc
--- /dev/null
+++ b/new_tko/tko/rpc_interface_unittest.py
@@ -0,0 +1,143 @@
+#!/usr/bin/python2.4
+
+import unittest
+import common
+from autotest_lib.new_tko import setup_django_environment
+from autotest_lib.frontend import setup_test_environment
+from django.db import connection
+from autotest_lib.new_tko.tko import models, rpc_interface
+
+# this will need to be updated when the view changes for the test to be
+# consistent with reality
+_CREATE_TEST_VIEW = """
+CREATE VIEW test_view_2 AS
+SELECT tests.test_idx AS test_idx,
+ tests.job_idx AS job_idx,
+ tests.test AS test_name,
+ tests.subdir AS subdir,
+ tests.kernel_idx AS kernel_idx,
+ tests.status AS status_idx,
+ tests.reason AS reason,
+ tests.machine_idx AS machine_idx,
+ tests.started_time AS test_started_time,
+ tests.finished_time AS test_finished_time,
+ jobs.tag AS job_tag,
+ jobs.label AS job_name,
+ jobs.username AS job_owner,
+ jobs.queued_time AS job_queued_time,
+ jobs.started_time AS job_started_time,
+ jobs.finished_time AS job_finished_time,
+ machines.hostname AS hostname,
+ machines.machine_group AS platform,
+ machines.owner AS machine_owner,
+ kernels.kernel_hash AS kernel_hash,
+ kernels.base AS kernel_base,
+ kernels.printable AS kernel,
+ status.word AS status
+FROM tests
+INNER JOIN jobs ON jobs.job_idx = tests.job_idx
+INNER JOIN machines ON machines.machine_idx = jobs.machine_idx
+INNER JOIN kernels ON kernels.kernel_idx = tests.kernel_idx
+INNER JOIN status ON status.status_idx = tests.status;
+"""
+
+def setup_test_view():
+ """
+ Django has no way to actually represent a view; we simply create a model for
+ TestView. This means when we syncdb, Django will create a table for it.
+ So manually remove that table and replace it with a view.
+ """
+ cursor = connection.cursor()
+ cursor.execute('DROP TABLE test_view_2')
+ cursor.execute(_CREATE_TEST_VIEW)
+
+
+class RpcInterfaceTest(unittest.TestCase):
+ def setUp(self):
+ setup_test_environment.set_up()
+ setup_test_view()
+ self._create_initial_data()
+
+
+ def tearDown(self):
+ setup_test_environment.tear_down()
+
+
+ def _create_initial_data(self):
+ machine = models.Machine(hostname='host1')
+ machine.save()
+
+ kernel_name = 'mykernel'
+ kernel = models.Kernel(kernel_hash=kernel_name, base=kernel_name,
+ printable=kernel_name)
+ kernel.save()
+
+ status = models.Status(word='GOOD')
+ status.save()
+
+ job = models.Job(tag='myjobtag', label='myjob', username='myuser',
+ machine=machine)
+ job.save()
+
+ test = models.Test(job=job, test='mytest', kernel=kernel,
+ status=status, machine=machine)
+ test.save()
+
+ attribute = models.TestAttribute(test=test, attribute='myattr',
+ value='myval')
+ attribute.save()
+
+ iteration_attribute = models.IterationAttribute(test=test, iteration=1,
+ attribute='iattr',
+ value='ival')
+ iteration_result = models.IterationResult(test=test, iteration=1,
+ attribute='iresult',
+ value=1)
+ iteration_attribute.save()
+ iteration_result.save()
+
+ test_label = models.TestLabel(name='testlabel')
+ test_label.save()
+ test_label.tests.add(test)
+
+
+ def test_get_detailed_test_views(self):
+ test = rpc_interface.get_detailed_test_views()[0]
+
+ self.assertEquals(test['test_name'], 'mytest')
+ self.assertEquals(test['job_tag'], 'myjobtag')
+ self.assertEquals(test['job_name'], 'myjob')
+ self.assertEquals(test['job_owner'], 'myuser')
+ self.assertEquals(test['status'], 'GOOD')
+ self.assertEquals(test['hostname'], 'host1')
+ self.assertEquals(test['kernel'], 'mykernel')
+
+ self.assertEquals(test['attributes'], {'myattr' : 'myval'})
+ self.assertEquals(test['iterations'], [{'attr' : {'iattr' : 'ival'},
+ 'perf' : {'iresult' : 1}}])
+ self.assertEquals(test['labels'], ['testlabel'])
+
+
+ def test_test_attributes(self):
+ rpc_interface.set_test_attribute('foo', 'bar', test_name='mytest')
+ test = rpc_interface.get_detailed_test_views()[0]
+ self.assertEquals(test['attributes'], {'foo' : 'bar',
+ 'myattr' : 'myval'})
+
+ rpc_interface.set_test_attribute('foo', 'goo', test_name='mytest')
+ test = rpc_interface.get_detailed_test_views()[0]
+ self.assertEquals(test['attributes'], {'foo' : 'goo',
+ 'myattr' : 'myval'})
+
+ rpc_interface.set_test_attribute('foo', None, test_name='mytest')
+ test = rpc_interface.get_detailed_test_views()[0]
+ self.assertEquals(test['attributes'], {'myattr' : 'myval'})
+
+
+ def test_immutable_attributes(self):
+ self.assertRaises(ValueError, rpc_interface.set_test_attribute,
+ 'myattr', 'foo', test_name='mytest')
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/tko/migrations/027_user_created_test_attributes.py b/tko/migrations/027_user_created_test_attributes.py
new file mode 100644
index 0000000..05767cb
--- /dev/null
+++ b/tko/migrations/027_user_created_test_attributes.py
@@ -0,0 +1,9 @@
+UP_SQL = """
+ALTER TABLE test_attributes
+ ADD COLUMN id integer NOT NULL AUTO_INCREMENT PRIMARY KEY,
+ ADD COLUMN user_created bool NOT NULL DEFAULT FALSE;
+"""
+
+DOWN_SQL = """
+ALTER TABLE test_attributes DROP COLUMN user_created, DROP COLUMN id;
+"""
diff --git a/utils/unittest_suite.py b/utils/unittest_suite.py
index a256b4c..91da532 100755
--- a/utils/unittest_suite.py
+++ b/utils/unittest_suite.py
@@ -24,6 +24,7 @@
'frontend_unittest.py',
'client_compilation_unittest.py',
'csv_encoder_unittest.py',
+ 'rpc_interface_unittest.py',
))
modules = []