fix lint error Created using spr 1.3.7
diff --git a/lnt/server/db/testsuitedb.py b/lnt/server/db/testsuitedb.py index 08552c5..a088c60 100644 --- a/lnt/server/db/testsuitedb.py +++ b/lnt/server/db/testsuitedb.py
@@ -1205,6 +1205,99 @@ self._importSampleValues(session, data['tests'], run, config) return run + def _importABRun(self, session, run_data, machine): + """Create and insert an ABRun for the given machine. + + No order tracking is performed; A/B runs are isolated from trend + analysis and regression detection. + """ + run_parameters = run_data.copy() + run_parameters.pop('id', None) + run_parameters.pop('order_by', None) + run_parameters.pop('order_id', None) + run_parameters.pop('machine_id', None) + + start_time_str = run_parameters.pop('start_time', None) + if start_time_str: + try: + start_time = aniso8601.parse_datetime(start_time_str) + except ValueError: + start_time = datetime.datetime.strptime(start_time_str, + "%Y-%m-%d %H:%M:%S") + else: + start_time = None + + end_time_str = run_parameters.pop('end_time', None) + if end_time_str: + try: + end_time = aniso8601.parse_datetime(end_time_str) + except ValueError: + end_time = datetime.datetime.strptime(end_time_str, + "%Y-%m-%d %H:%M:%S") + else: + end_time = None + + run = self.ABRun() + run.machine = machine + run.start_time = start_time + run.end_time = end_time + for item in self.run_fields: + value = run_parameters.pop(item.name, None) + run.set_field(item, value) + run.parameters = run_parameters + session.add(run) + return run + + def _importABSampleValues(self, session, tests_data, run): + """Create ABSample rows for the given tests data and ABRun. + + Mirrors _importSampleValues but creates ABSample objects and skips + profile handling. + """ + test_cache = dict((test.name, test) + for test in session.query(self.Test)) + field_dict = dict([(f.name, f) for f in self.sample_fields]) + all_samples = [] + + for test_data in tests_data: + name = test_data['name'] + test = test_cache.get(name) + if test is None: + test = self.Test(test_data['name']) + test_cache[name] = test + session.add(test) + + samples = [] + for key, values in test_data.items(): + if key == 'name' or key == 'id' or key.endswith('_id'): + continue + field = field_dict.get(key) + if field is None: + raise ValueError("test %s: Metric %r unknown in suite" % + (name, key)) + if not isinstance(values, list): + values = [values] + while len(samples) < len(values): + sample = self.ABSample() + sample.run = run + sample.test = test + samples.append(sample) + all_samples.append(sample) + for sample, value in zip(samples, values): + sample.set_field(field, value) + + session.add_all(all_samples) + + def importABDataFromDict(self, session, data): + """Import a report into ABRun/ABSample tables. + + No order or regression tracking is performed. + """ + machine = self._getOrCreateMachine(session, data['machine'], 'match') + run = self._importABRun(session, data['run'], machine) + self._importABSampleValues(session, data['tests'], run) + return run + # Simple query support (mostly used by templates) def machines(self, session, name=None):
diff --git a/lnt/server/reporting/analysis.py b/lnt/server/reporting/analysis.py index 8d18ec0..565d1ae 100644 --- a/lnt/server/reporting/analysis.py +++ b/lnt/server/reporting/analysis.py
@@ -419,3 +419,29 @@ self.profile_map[(run_id, test_id)] = profile_id self.loaded_run_ids |= to_load + + +class ABRunInfo(RunInfo): + """Like RunInfo but queries ABSample instead of Sample. + + Uses getattr(ts.ABSample, f.name) to reference columns rather than + f.column, which points to the Sample table. + """ + + def _load_samples_for_runs(self, session, run_ids, only_tests): + to_load = set(run_ids) - self.loaded_run_ids + if not to_load: + return + + ABSample = self.testsuite.ABSample + columns = [ABSample.run_id, ABSample.test_id] + columns.extend( + getattr(ABSample, f.name) for f in self.testsuite.sample_fields) + q = session.query(*columns) + if only_tests: + q = q.filter(ABSample.test_id.in_(only_tests)) + q = q.filter(ABSample.run_id.in_(to_load)) + for (run_id, test_id, *sample_values) in q: + self.sample_map[(run_id, test_id)] = sample_values + + self.loaded_run_ids |= to_load
diff --git a/lnt/server/ui/api.py b/lnt/server/ui/api.py index 00119c9..b1998ec 100644 --- a/lnt/server/ui/api.py +++ b/lnt/server/ui/api.py
@@ -1,3 +1,4 @@ +import datetime import lnt.util.ImportData import re import yaml @@ -9,6 +10,7 @@ from sqlalchemy.orm import joinedload from sqlalchemy.orm.exc import NoResultFound +from lnt.server.reporting.analysis import ABRunInfo from lnt.server.ui.util import convert_revision from lnt.server.ui.decorators import in_db from lnt.testing import PASS @@ -589,6 +591,143 @@ return results +def _ab_exp_to_dict(exp): + return { + 'id': exp.id, + 'name': exp.name, + 'created_time': str(exp.created_time), + 'pinned': exp.pinned, + 'control_run_id': exp.control_run_id, + 'variant_run_id': exp.variant_run_id, + } + + +class ABTests(Resource): + """List or create A/B experiments for a test suite.""" + method_decorators = [in_db] + + @staticmethod + @requires_auth_token + def post(): + """Submit control and variant reports and create an ABExperiment.""" + session = request.session + ts = request.get_testsuite() + body = request.get_json(force=True) + if body is None: + abort(400, msg="Request body must be JSON.") + for key in ('control', 'variant'): + if key not in body: + abort(400, msg="Missing required field: %r" % key) + control_run = ts.importABDataFromDict(session, body['control']) + session.flush() + variant_run = ts.importABDataFromDict(session, body['variant']) + session.flush() + exp = ts.ABExperiment() + exp.name = body.get('name', '') + exp.created_time = datetime.datetime.utcnow() + exp.control_run_id = control_run.id + exp.variant_run_id = variant_run.id + exp.pinned = bool(body.get('pinned', False)) + session.add(exp) + session.commit() + new_url = ('%sapi/db_%s/v4/%s/abtest/%s' % + (request.url_root, g.db_name, g.testsuite_name, exp.id)) + result = _ab_exp_to_dict(exp) + result['url'] = new_url + response = jsonify(result) + response.status = '201' + response.headers.add('Location', new_url) + return response + + @staticmethod + def get(): + """List A/B experiments, optionally filtered by pinned status.""" + session = request.session + ts = request.get_testsuite() + q = session.query(ts.ABExperiment).order_by( + ts.ABExperiment.created_time.desc()) + pinned = request.args.get('pinned') + if pinned is not None: + q = q.filter(ts.ABExperiment.pinned == (pinned == 'true')) + limit = int(request.args.get('limit', 50)) + offset = int(request.args.get('offset', 0)) + exps = q.limit(limit).offset(offset).all() + return {'experiments': [_ab_exp_to_dict(e) for e in exps]} + + +class ABTestDetail(Resource): + """Retrieve or update a single A/B experiment.""" + method_decorators = [in_db] + + @staticmethod + def get(abtest_id): + """Return the experiment metadata and per-test comparison results.""" + session = request.session + ts = request.get_testsuite() + exp = session.query(ts.ABExperiment).filter_by(id=abtest_id).first() + if exp is None: + abort(404, msg="No such A/B experiment.") + + control_run = session.query(ts.ABRun).filter_by( + id=exp.control_run_id).first() + variant_run = session.query(ts.ABRun).filter_by( + id=exp.variant_run_id).first() + if control_run is None or variant_run is None: + abort(404, msg="Experiment references a missing ABRun.") + + sri = ABRunInfo(session, ts, + [exp.control_run_id, exp.variant_run_id]) + test_ids = sri.test_ids + test_rows = session.query(ts.Test.id, ts.Test.name).filter( + ts.Test.id.in_(test_ids)).all() + test_name_map = {tid: name for tid, name in test_rows} + + metric_fields = [f for f in ts.sample_fields + if f.type.name in ('Real', 'Integer')] + + comparisons = [] + for test_id in sorted(test_ids): + test_name = test_name_map.get(test_id, str(test_id)) + for field in metric_fields: + cr = sri.get_run_comparison_result( + variant_run, control_run, test_id, field, None) + if cr.current is None and cr.previous is None: + continue + comparisons.append({ + 'test_name': test_name, + 'field': field.name, + 'control': cr.previous, + 'variant': cr.current, + 'pct_delta': round(cr.pct_delta * 100, 4) + if cr.pct_delta is not None else None, + 'status': cr.get_value_status(), + }) + + return { + 'experiment': _ab_exp_to_dict(exp), + 'comparisons': comparisons, + } + + @staticmethod + @requires_auth_token + def patch(abtest_id): + """Update mutable fields (currently: pinned) of an ABExperiment.""" + session = request.session + ts = request.get_testsuite() + exp = session.query(ts.ABExperiment).filter_by(id=abtest_id).first() + if exp is None: + abort(404, msg="No such A/B experiment.") + body = request.get_json(force=True) + if body is None: + abort(400, msg="Request body must be JSON.") + if 'pinned' in body: + exp.pinned = bool(body['pinned']) + if 'name' in body: + exp.name = body['name'] + session.commit() + return _ab_exp_to_dict(exp) + + def ts_path(path): """Make a URL path with a database and test suite embedded in them.""" return "/api/db_<string:db>/v4/<string:ts>/" + path @@ -619,3 +758,5 @@ regression_url = \ "regression/<int:machine_id>/<int:test_id>/<int:field_index>" api.add_resource(Regression, ts_path(regression_url)) + api.add_resource(ABTests, ts_path("abtest"), ts_path("abtest/")) + api.add_resource(ABTestDetail, ts_path("abtest/<int:abtest_id>"))
diff --git a/tests/server/ui/test_abtest_api.py b/tests/server/ui/test_abtest_api.py new file mode 100644 index 0000000..32c7a22 --- /dev/null +++ b/tests/server/ui/test_abtest_api.py
@@ -0,0 +1,201 @@ +# Check the A/B testing REST API endpoints. +# RUN: rm -rf %t.instance +# RUN: python %{shared_inputs}/create_temp_instance.py \ +# RUN: %s %{shared_inputs}/SmallInstance \ +# RUN: %t.instance %S/Inputs/V4Pages_extra_records.sql +# +# RUN: python %s %t.instance + +import json +import logging +import sys +import unittest + +import lnt.server.db.migrate +import lnt.server.ui.app + +logging.basicConfig(level=logging.INFO) + +AUTH_TOKEN = 'test_token' +BASE_URL = 'api/db_default/v4/nts/' + +CONTROL_DATA = { + 'machine': {'name': 'ab-test-machine', 'hardware': 'x86_64', 'os': 'linux'}, + 'run': { + 'start_time': '2024-01-01T00:00:00', + 'end_time': '2024-01-01T00:05:00', + }, + 'tests': [ + {'name': 'SingleSource/Benchmarks/Misc/mandelbrot', + 'compile_time': 1.0, 'execution_time': 2.0}, + {'name': 'SingleSource/Benchmarks/Misc/lowercase', + 'compile_time': 0.5}, + ], +} + +VARIANT_DATA = { + 'machine': {'name': 'ab-test-machine', 'hardware': 'x86_64', 'os': 'linux'}, + 'run': { + 'start_time': '2024-01-01T00:10:00', + 'end_time': '2024-01-01T00:15:00', + }, + 'tests': [ + {'name': 'SingleSource/Benchmarks/Misc/mandelbrot', + 'compile_time': 1.05, 'execution_time': 1.95}, + {'name': 'SingleSource/Benchmarks/Misc/lowercase', + 'compile_time': 0.48}, + ], +} + + +class ABTestAPITest(unittest.TestCase): + def setUp(self): + _, instance_path = sys.argv + app = lnt.server.ui.app.App.create_standalone(instance_path) + app.testing = True + self.client = app.test_client() + + def _post_json(self, url, body, token=AUTH_TOKEN): + return self.client.post(url, data=json.dumps(body), + content_type='application/json', + headers={'AuthToken': token}) + + def _patch_json(self, url, body, token=AUTH_TOKEN): + return self.client.patch(url, data=json.dumps(body), + content_type='application/json', + headers={'AuthToken': token}) + + def test_01_create_experiment(self): + """POST /abtest creates an experiment and returns 201.""" + body = { + 'name': 'test-experiment', + 'control': CONTROL_DATA, + 'variant': VARIANT_DATA, + } + resp = self._post_json(BASE_URL + 'abtest', body) + self.assertEqual(resp.status_code, 201, + "Expected 201, got %d: %s" % + (resp.status_code, resp.data)) + result = json.loads(resp.data) + self.assertIn('id', result) + self.assertIn('url', result) + self.assertEqual(result['name'], 'test-experiment') + self.assertFalse(result['pinned']) + self.exp_id = result['id'] + + def test_02_get_experiment_detail(self): + """GET /abtest/<id> returns comparison results.""" + # First create an experiment. + body = { + 'name': 'detail-test', + 'control': CONTROL_DATA, + 'variant': VARIANT_DATA, + } + resp = self._post_json(BASE_URL + 'abtest', body) + self.assertEqual(resp.status_code, 201) + exp_id = json.loads(resp.data)['id'] + + # Now GET the detail. + resp = self.client.get(BASE_URL + 'abtest/%d' % exp_id) + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data) + self.assertIn('experiment', result) + self.assertIn('comparisons', result) + self.assertEqual(result['experiment']['name'], 'detail-test') + # Both tests reported compile_time; expect two compile_time comparisons. + cmp_by_field = {(c['test_name'], c['field']): c + for c in result['comparisons']} + key = ('SingleSource/Benchmarks/Misc/mandelbrot', 'compile_time') + self.assertIn(key, cmp_by_field, + "Expected mandelbrot/compile_time in comparisons") + entry = cmp_by_field[key] + self.assertAlmostEqual(entry['control'], 1.0) + self.assertAlmostEqual(entry['variant'], 1.05) + + def test_03_patch_pinned(self): + """PATCH /abtest/<id> updates the pinned field.""" + body = { + 'name': 'pin-test', + 'control': CONTROL_DATA, + 'variant': VARIANT_DATA, + } + resp = self._post_json(BASE_URL + 'abtest', body) + self.assertEqual(resp.status_code, 201) + exp_id = json.loads(resp.data)['id'] + + resp = self._patch_json(BASE_URL + 'abtest/%d' % exp_id, + {'pinned': True}) + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data) + self.assertTrue(result['pinned']) + + # Verify the change is persisted. + resp = self.client.get(BASE_URL + 'abtest/%d' % exp_id) + self.assertEqual(resp.status_code, 200) + detail = json.loads(resp.data) + self.assertTrue(detail['experiment']['pinned']) + + def test_04_list_experiments(self): + """GET /abtest lists experiments.""" + body = { + 'name': 'list-test', + 'control': CONTROL_DATA, + 'variant': VARIANT_DATA, + } + self._post_json(BASE_URL + 'abtest', body) + + resp = self.client.get(BASE_URL + 'abtest') + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data) + self.assertIn('experiments', result) + self.assertGreaterEqual(len(result['experiments']), 1) + names = [e['name'] for e in result['experiments']] + self.assertIn('list-test', names) + + def test_05_list_filter_pinned(self): + """GET /abtest?pinned=true returns only pinned experiments.""" + # Create one pinned and one unpinned experiment. + for name, pinned in [('pinned-exp', True), ('unpinned-exp', False)]: + body = { + 'name': name, + 'pinned': pinned, + 'control': CONTROL_DATA, + 'variant': VARIANT_DATA, + } + resp = self._post_json(BASE_URL + 'abtest', body) + self.assertEqual(resp.status_code, 201) + + resp = self.client.get(BASE_URL + 'abtest?pinned=true') + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data) + self.assertTrue(all(e['pinned'] for e in result['experiments'])) + + def test_06_missing_control_returns_400(self): + """POST without control field returns 400.""" + body = {'name': 'bad', 'variant': VARIANT_DATA} + resp = self._post_json(BASE_URL + 'abtest', body) + self.assertEqual(resp.status_code, 400) + + def test_07_unknown_experiment_returns_404(self): + """GET /abtest/99999 returns 404 for a missing experiment.""" + resp = self.client.get(BASE_URL + 'abtest/99999') + self.assertEqual(resp.status_code, 404) + + def test_08_patch_requires_auth(self): + """PATCH without auth token returns 401.""" + body = { + 'name': 'auth-test', + 'control': CONTROL_DATA, + 'variant': VARIANT_DATA, + } + resp = self._post_json(BASE_URL + 'abtest', body) + self.assertEqual(resp.status_code, 201) + exp_id = json.loads(resp.data)['id'] + + resp = self._patch_json(BASE_URL + 'abtest/%d' % exp_id, + {'pinned': True}, token='wrong_token') + self.assertEqual(resp.status_code, 401) + + +if __name__ == '__main__': + unittest.main(argv=sys.argv[:1])