Differential D372

squash_results: fix crash for incomplete updates
ClosedPublic
Actions

Authored by kparal on May 12 2015, 2:23 PM.

Details

Reviewers
tflink
mkrizek
jskladan
Maniphest Tasks
T476: KeyError crash in _squash_builds_to_updates
Commits
rDEPCKa836e604b34c: squash_results: fix crash for incomplete updates
Summary

A bug was introduced in edf28defd6 which caused a crash when a missing
build was processed as the first build of an incomplete update. This patch
fixes that.

It also removes some stray comments and improves debug messages to be
more helpful.

Test Plan

test suite works and manual depcheck run works

Diff Detail

Repository
rDEPCK task-depcheck
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal retitled this revision from to squash_results: fix crash for incomplete updates.May 12 2015, 2:23 PM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added reviewers: tflink, jskladan, mkrizek.
kparal added a task: T476: KeyError crash in _squash_builds_to_updates.
jskladan accepted this revision.May 13 2015, 8:43 AM
This revision is now accepted and ready to land.May 13 2015, 8:43 AM
Closed by commit rDEPCKa836e604b34c: squash_results: fix crash for incomplete updates (authored by kparal). · Explain WhyMay 13 2015, 11:15 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathPackages
Mdepcheck.yml (2 lines)
Mdepcheck/depcheck.py (5 lines)
Mdepcheck/squash_results.py (50 lines)
Mtests/test_squash_results.py (36 lines)

Diff 987

View Options

depcheck.yml

Show All 26 Lines24 mash:
27 arch: ['i386'] 27​ arch: ['i386']
28 28​
29 - name: get yum repos 29​ - name: get yum repos
30 yumrepoinfo: 30​ yumrepoinfo:
31 koji_tag: ${koji_tag} 31​ koji_tag: ${koji_tag}
32 arch: ['i386'] 32​ arch: ['i386']
33 export: yumrepos_i386 33​ export: yumrepos_i386
34 34​
35 # get repos ​
36 - name: run depcheck 35​ - name: run depcheck
37 python: 36​ python:
38 file: run_depcheck.py 37​ file: run_depcheck.py
39 callable: taskotron_run 38​ callable: taskotron_run
40 arch: ['i386'] 39​ arch: ['i386']
41 rpms: ["${workdir}/downloaded_tag/"] 40​ rpms: ["${workdir}/downloaded_tag/"]
42 repos: 41​ repos:
43 - ${yumrepos_i386} 42​ - ${yumrepos_i386}
Show All 16 Lines56 mash:
60 arch: ['x86_64'] 59​ arch: ['x86_64']
61 60​
62 - name: get yum repos 61​ - name: get yum repos
63 yumrepoinfo: 62​ yumrepoinfo:
64 koji_tag: ${koji_tag} 63​ koji_tag: ${koji_tag}
65 arch: ['x86_64'] 64​ arch: ['x86_64']
66 export: yumrepos_x86_64 65​ export: yumrepos_x86_64
67 66​
68 # get repos ​
69 - name: run depcheck 67​ - name: run depcheck
70 python: 68​ python:
71 file: run_depcheck.py 69​ file: run_depcheck.py
72 callable: taskotron_run 70​ callable: taskotron_run
73 arch: ['x86_64'] 71​ arch: ['x86_64']
74 rpms: ["${workdir}/downloaded_tag/"] 72​ rpms: ["${workdir}/downloaded_tag/"]
75 repos: 73​ repos:
76 - ${yumrepos_x86_64} 74​ - ${yumrepos_x86_64}
Show All 28 Lines
View Options

depcheck/depcheck.py

Show First 20 Lines • Show All 129 Lines • ▼ Show 20 Line(s)125apologize for the inconvenience and are working to get it fixed as soon as possible."""
130 def check_rpm_deps(self, rpmfiles): 130​ def check_rpm_deps(self, rpmfiles):
131 """Check dependencies of the given rpms against supplied repodata 131​ """Check dependencies of the given rpms against supplied repodata
132 132​
133 :param rpmfiles: local absolute paths to rpms to check 133​ :param rpmfiles: local absolute paths to rpms to check
134 :type rpmfiles: list of str 134​ :type rpmfiles: list of str
135 :returns: dictionary of results {rpm: {'results': Pass/Fail, 'details': []}} 135​ :returns: dictionary of results {rpm: {'results': Pass/Fail, 'details': []}}
136 """ 136​ """
137 137​
138 log.logger.debug('adding all rpms to libsolv repos and preparing solvpool') 138​ log.logger.debug('adding %d rpms to libsolv repos and preparing solvpool',
139​ len(rpmfiles))
139 140​
140 141​
141 # add all rpmfiles that we're checking to the libsolv repo and save the 142​ # add all rpmfiles that we're checking to the libsolv repo and save the
142 # solvable that is assigned to them. 143​ # solvable that is assigned to them.
143 # this makes libsolv aware of all the incoming rpms and allows it to 144​ # this makes libsolv aware of all the incoming rpms and allows it to
144 # check for any dep breakage between the rpms 145​ # check for any dep breakage between the rpms
145 self._prepare_libsolv() 146​ self._prepare_libsolv()
146 solvables = {} 147​ solvables = {}
147 for rpmfile in rpmfiles: 148​ for rpmfile in rpmfiles:
148 rpmname = rpmfile.split('/')[-1] 149​ rpmname = rpmfile.split('/')[-1]
149 solvable = self.solvrepo.add_rpm(rpmfile) 150​ solvable = self.solvrepo.add_rpm(rpmfile)
150 solvables[rpmname] = solvable 151​ solvables[rpmname] = solvable
151 152​
152 # now that the rpms have been added, prepare the solvpool 153​ # now that the rpms have been added, prepare the solvpool
153 self.solvpool.addfileprovides() 154​ self.solvpool.addfileprovides()
154 self.solvpool.createwhatprovides() 155​ self.solvpool.createwhatprovides()
155 156​
156 157​
157 # check each rpm using its solvable that was assigned while adding to 158​ # check each rpm using its solvable that was assigned while adding to
158 # the repo 159​ # the repo
159 results = {} 160​ results = {}
160 for rpmname in solvables: 161​ for rpmname in sorted(solvables.keys()):
161 log.logger.debug('running depcheck for %s' % rpmname) 162​ log.logger.debug('running depcheck for %s' % rpmname)
162 run_result = self._check_rpm(rpmname, solvables[rpmname]) 163​ run_result = self._check_rpm(rpmname, solvables[rpmname])
163 results[rpmname] = run_result 164​ results[rpmname] = run_result
164 165​
165 return results 166​ return results
166 167​
167 168​
View Options

depcheck/squash_results.py

Show First 20 Lines • Show All 66 Lines • ▼ Show 20 Line(s)66 if update['title'] not in updates:
67 updates[update['title']] = update 67​ updates[update['title']] = update
68 68​
69 updates_results = [] 69​ updates_results = []
70 for update_title, update in updates.items(): 70​ for update_title, update in updates.items():
71 cd = check.CheckDetail(update_title, check.ReportType.BODHI_UPDATE, outcome = 'PASSED') 71​ cd = check.CheckDetail(update_title, check.ReportType.BODHI_UPDATE, outcome = 'PASSED')
72 updates_results.append(cd) 72​ updates_results.append(cd)
73 73​
74 builds = sorted([build['nvr'] for build in update['builds']]) 74​ builds = sorted([build['nvr'] for build in update['builds']])
75 for build in builds: 75​
76 if build in pending_fail.keys(): 76​ if set(builds).intersection(set(pending_fail.keys())):
77 # A build is in pending_fail either when it has no assigned 77​ # A build is in pending_fail either when it has no assigned
78 # update (that can not happen, as we traverse builds of 78​ # update (that can not happen, as we traverse builds of
79 # a specific update), or when an update was incomplete 79​ # a specific update), or when an update was incomplete
80 # (e.g. missing some builds). 80​ # (e.g. missing some builds).
81 # If an update is incomplete, all its builds are put 81​ # If an update is incomplete, all its builds are put
82 # into the pending_fail, so this should get detected immediately. 82​ # into the pending_fail.
83 cd.update_outcome('ABORTED') 83​ cd.update_outcome('ABORTED')
84 cd.store('Update was incomplete as downloaded at execution ' 84​ cd.store('Update was incomplete as downloaded at execution '
85 'time and not all builds have been tested. This can ' 85​ 'time and not all builds have been tested. This can '
86 'happen if the update is edited in Bodhi during check ' 86​ 'happen if the update is edited in Bodhi during check '
87 'execution.', printout=False) 87​ 'execution.', printout=False)
88 expected = set(builds) 88​ expected = set(builds)
89 tested = set([bld for bld, upd in pending_fail.items() if upd is update]) 89​ tested = set([bld for bld, upd in pending_fail.items() if upd is update])
90 missing = expected.difference(tested) 90​ missing = expected.difference(tested)
91 cd.store("Builds which haven't been tested:", printout=False) 91​ cd.store("Builds which haven't been tested:", printout=False)
92 for bld in sorted(missing): 92​ for bld in sorted(missing):
93 cd.store(' %s' % bld, printout=False) 93​ cd.store(' %s' % bld, printout=False)
94 break 94​ else:
95 95​ for build in builds:
96 if results_nvr[build]['result'] != Pass: 96​ if results_nvr[build]['result'] != Pass:
97 cd.update_outcome(results_nvr[build]['result']) 97​ cd.update_outcome(results_nvr[build]['result'])
98 cd.store('Build %s failed depcheck' % build, printout=False) 98​ cd.store('Build %s failed depcheck' % build, printout=False)
99 cd.output.extend(results_nvr[build]['details']) 99​ cd.output.extend(results_nvr[build]['details'])
100 100​
101 # set summary 101​ # set summary
102 if update['request']: 102​ if update['request']:
103 target = 'into %s %s' % (update['release']['name'], 103​ target = 'into %s %s' % (update['release']['name'],
104 update['request']) 104​ update['request'])
105 else: 105​ else:
106 target = 'in %s %s' % (update['release']['name'], 106​ target = 'in %s %s' % (update['release']['name'],
107 update['status']) 107​ update['status'])
Show All 40 Lines
View Options

tests/test_squash_results.py

Show First 20 Lines • Show All 118 Lines • ▼ Show 20 Line(s)117 update = {'title': 'update_name',
119 'release': {'name': 'F20'}, 119​ 'release': {'name': 'F20'},
120 'request': 'stable', 120​ 'request': 'stable',
121 } 121​ }
122 update['builds'].append({'nvr':'nvr3'}) 122​ update['builds'].append({'nvr':'nvr3'})
123 build_2_update = ({}, 123​ build_2_update = ({},
124 { 124​ {
125 'nvr1': update, 125​ 'nvr1': update,
126 'nvr2': update, 126​ 'nvr2': update,
127 'nvr3': update, 127​ })
128​
129​ fake_bodhi = Dingus(build2update__returns = build_2_update)
130​
131​ cds = _squash_builds_to_updates(run_results, fake_bodhi)
132​ assert len(cds) == 1
133​
134​ detail = cds[0]
135​ assert detail.item == update['title']
136​ assert detail.outcome == 'ABORTED'
137​ assert any('Update was incomplete' in line for line in detail.output)
138​
139​ def test__squash_builds_to_updates_incomplete_update_missing_first(self):
140​ """
141​ Checks whether __squash_builds_to_updates provides correct output
142​ when some update is incomplete (i.e. present in the second return-value
143​ from the build2update method) and the missing build is the first one
144​ to process (https://phab.qadevel.cloud.fedoraproject.org/T476).
145​ """
146​ run_results = {
147​ 'nvr1': {'result': Pass, 'details': []},
148​ 'nvr2': {'result': Pass, 'details': []},
149​ }
150​ update = {'title': 'update_name',
151​ 'builds': [{'nvr': k} for k in run_results.keys()],
152​ 'release': {'name': 'F20'},
153​ 'request': 'stable',
154​ }
155​ # the builds are processed alphabetically in _squash_build_to_updates(),
156​ # so naming it 'abc1' ensures it is processed before 'nvr*'
157​ update['builds'].append({'nvr':'abc1'})
158​ build_2_update = ({},
159​ {
160​ 'nvr1': update,
161​ 'nvr2': update,
128 }) 162​ })
129 163​
130 fake_bodhi = Dingus(build2update__returns = build_2_update) 164​ fake_bodhi = Dingus(build2update__returns = build_2_update)
131 165​
132 cds = _squash_builds_to_updates(run_results, fake_bodhi) 166​ cds = _squash_builds_to_updates(run_results, fake_bodhi)
133 assert len(cds) == 1 167​ assert len(cds) == 1
134 168​
135 detail = cds[0] 169​ detail = cds[0]
Show All 32 Lines