Skip to content
This repository has been archived by the owner on Dec 15, 2018. It is now read-only.

Commit

Permalink
Remove the 'changed tests' info from the build details API
Browse files Browse the repository at this point in the history
Summary:
Data isn't exposed in the UI or used by any API clients I'm aware of, and at a spot-check wasn't
correct. But even if it were correct, it's taking up >1/3 of the build api endpoint for data we
aren't using, so seems best to remove it and look to our history if we want to revive.

Test Plan: Changes

Reviewers: naphat

Reviewed By: naphat

Subscribers: changesbot, anupc

Differential Revision: https://tails.corp.dropbox.com/D227879
  • Loading branch information
kylec1 committed Sep 12, 2016
1 parent c51411d commit a0162d7
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 164 deletions.
114 changes: 0 additions & 114 deletions changes/api/build_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from collections import defaultdict
from flask_restful.reqparse import RequestParser
from itertools import groupby
from sqlalchemy import text
from sqlalchemy.orm import contains_eager, joinedload, subqueryload_all
from typing import List # NOQA

Expand All @@ -25,95 +24,6 @@
from changes.utils.originfinder import find_failure_origins


def find_changed_tests(current_build, previous_build, limit=25):
current_job_ids = [j.id.hex for j in current_build.jobs]
previous_job_ids = [j.id.hex for j in previous_build.jobs]

if not (current_job_ids and previous_job_ids):
return []

current_job_clause = ', '.join(
':c_job_id_%s' % i for i in range(len(current_job_ids))
)
previous_job_clause = ', '.join(
':p_job_id_%s' % i for i in range(len(previous_job_ids))
)

params = {}
for idx, job_id in enumerate(current_job_ids):
params['c_job_id_%s' % idx] = job_id
for idx, job_id in enumerate(previous_job_ids):
params['p_job_id_%s' % idx] = job_id

# find all tests that have appeared in one job but not the other
# we have to build this query up manually as sqlalchemy doesnt support
# the FULL OUTER JOIN clause
query = """
SELECT c.id AS c_id,
p.id AS p_id
FROM (
SELECT label_sha, id
FROM test
WHERE job_id IN (%(current_job_clause)s)
) as c
FULL OUTER JOIN (
SELECT label_sha, id
FROM test
WHERE job_id IN (%(previous_job_clause)s)
) as p
ON c.label_sha = p.label_sha
WHERE (c.id IS NULL OR p.id IS NULL)
""" % {
'current_job_clause': current_job_clause,
'previous_job_clause': previous_job_clause
}

total = db.session.query(
'count'
).from_statement(
text('SELECT COUNT(*) FROM (%s) as a' % (query,))
).params(**params).scalar()

if not total:
return {
'total': 0,
'changes': [],
}

results = db.session.query(
'c_id', 'p_id'
).from_statement(
text('%s LIMIT %d' % (query, limit))
).params(**params)

all_test_ids = set()
for c_id, p_id in results:
if c_id:
all_test_ids.add(c_id)
else:
all_test_ids.add(p_id)

test_map = dict(
(t.id, t) for t in TestCase.query.filter(
TestCase.id.in_(all_test_ids),
).options(
joinedload('job', innerjoin=True),
)
)

diff = []
for c_id, p_id in results:
if p_id:
diff.append(('-', test_map[p_id]))
else:
diff.append(('+', test_map[c_id]))

return {
'total': total,
'changes': sorted(diff, key=lambda x: (x[1].package, x[1].name)),
}


def get_failure_reasons(build):
from changes.buildfailures import registry

Expand Down Expand Up @@ -184,23 +94,6 @@ def get(self, build_id):
if build is None:
return '', 404

try:
most_recent_run = Build.query.filter(
Build.project == build.project,
Build.date_created < build.date_created,
Build.status == Status.finished,
Build.id != build.id,
).join(
Source, Build.source_id == Source.id,
).filter(
*build_type.get_any_commit_build_filters()
).options(
contains_eager('source').joinedload('revision'),
joinedload('author'),
).order_by(Build.date_created.desc())[0]
except IndexError:
most_recent_run = None

jobs = list(Job.query.filter(
Job.build_id == build.id,
))
Expand Down Expand Up @@ -230,12 +123,6 @@ def get(self, build_id):
for test_failure in test_failures:
test_failure.origin = failure_origins.get(test_failure)

# identify added/removed tests
if most_recent_run and build.status == Status.finished:
changed_tests = find_changed_tests(build, most_recent_run)
else:
changed_tests = []

seen_by = list(User.query.join(
BuildSeen, BuildSeen.user_id == User.id,
).filter(
Expand All @@ -261,7 +148,6 @@ def get(self, build_id):
'total': num_test_failures,
'tests': self.serialize(test_failures, extended_serializers),
},
'testChanges': self.serialize(changed_tests, extended_serializers),
'parents': self.serialize(get_parents_last_builds(build)),
})

Expand Down
51 changes: 1 addition & 50 deletions tests/changes/api/test_build_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,55 +6,7 @@
from changes.models.event import Event
from changes.models.failurereason import FailureReason
from changes.models.itemstat import ItemStat
from changes.models.test import TestCase
from changes.testutils import APITestCase, TestCase as BaseTestCase
from changes.api.build_details import find_changed_tests


class FindChangedTestsTest(BaseTestCase):
def test_simple(self):
project = self.create_project()

previous_build = self.create_build(project)
previous_job = self.create_job(previous_build)

changed_test = TestCase(
job=previous_job,
project=previous_job.project,
name='unchanged test',
)
removed_test = TestCase(
job=previous_job,
project=previous_job.project,
name='removed test',
)
db.session.add(removed_test)
db.session.add(changed_test)

current_build = self.create_build(project)
current_job = self.create_job(current_build)
added_test = TestCase(
job=current_job,
project=current_job.project,
name='added test',
)

db.session.add(added_test)
db.session.add(TestCase(
job=current_job,
project=current_job.project,
name='unchanged test',
))
db.session.commit()

results = find_changed_tests(current_build, previous_build)

assert results['total'] == 2

assert ('-', removed_test) in results['changes']
assert ('+', added_test) in results['changes']

assert len(results['changes']) == 2
from changes.testutils import APITestCase


class BuildDetailsTest(APITestCase):
Expand Down Expand Up @@ -99,7 +51,6 @@ def test_simple(self):
assert data['seenBy'] == []
assert data['testFailures']['total'] == 0
assert data['testFailures']['tests'] == []
assert data['testChanges'] == []
assert len(data['events']) == 1
assert len(data['failures']) == 1
assert data['failures'][0] == {
Expand Down

0 comments on commit a0162d7

Please sign in to comment.