diff --git a/osc-staging.py b/osc-staging.py index e59723f1..39cefbd2 100644 --- a/osc-staging.py +++ b/osc-staging.py @@ -124,7 +124,7 @@ def do_staging(self, subcmd, opts, *args): elif cmd == 'select': tprj = api.prj_from_letter(args[1]) if opts.add: - api.mark_additional_packages(tprj, [ opts.add ] ) + api.mark_additional_packages(tprj, [opts.add]) else: SelectCommand(api).perform(tprj, args[2:], opts.move, opts.from_) elif cmd == 'cleanup_rings': diff --git a/osclib/check_command.py b/osclib/check_command.py index 1aa54f96..c969eea3 100644 --- a/osclib/check_command.py +++ b/osclib/check_command.py @@ -90,10 +90,11 @@ class CheckCommand(object): report.append(' -- For subproject %s' % subproject['name']) report.extend(subreport) - if report and not is_subproject: - report.insert(0, ' -- Project %s still needs attention' % project['name']) - elif not is_subproject: - report.append(' ++ Acceptable staging project %s' % project['name']) + if project['overall_state'] == 'acceptable': + report.insert(0, ' ++ Acceptable staging project %s' % project['name']) + elif project['overall_state'] != 'empty': + report.insert(0, ' -- %s Project %s still needs attention' % (project['overall_state'].upper(), + project['name'])) return report diff --git a/osclib/comments.py b/osclib/comments.py index 64dc4ca1..e8a695ac 100644 --- a/osclib/comments.py +++ b/osclib/comments.py @@ -76,8 +76,8 @@ class CommentAPI(object): root = root = ET.parse(http_GET(url)).getroot() comments = {} for c in root.findall('comment'): - c = self._comment_as_dict(c) - comments[c['id']] = c + c = self._comment_as_dict(c) + comments[c['id']] = c return comments def add_comment(self, request_id=None, project_name=None, @@ -105,15 +105,15 @@ class CommentAPI(object): def delete_children(self, comments): """Removes the comments that have no childs - + :param comments dict of id->comment dict - :return same hash without the deleted comments + :return same hash without the deleted comments """ parents = [] for comment in comments.values(): if comment['parent']: parents.append(comment['parent']) - + for id_ in comments.keys(): if id_ not in parents: self.delete(id_) @@ -135,7 +135,7 @@ class CommentAPI(object): return True def delete_from_where_user(self, user, request_id=None, project_name=None, - package_name=None): + package_name=None): """Remove comments where @user is mentioned. This method is used to remove notifications when a request is diff --git a/osclib/stagingapi.py b/osclib/stagingapi.py index b37d7e55..72a155ee 100644 --- a/osclib/stagingapi.py +++ b/osclib/stagingapi.py @@ -6,7 +6,6 @@ import json import logging -import re import urllib2 import time from xml.etree import cElementTree as ET @@ -475,7 +474,7 @@ class StagingAPI(object): for sub_prj, sub_pkg in self.get_sub_packages(package): sub_prj = self.map_ring_package_to_subject(project, sub_pkg) - if sub_prj != subprj: # if different to the main package's prj + if sub_prj != subprj: # if different to the main package's prj delete_package(self.apiurl, sub_prj, sub_pkg, force=True, msg=msg) self.set_review(request_id, project, state=review, msg=msg) @@ -499,48 +498,6 @@ class StagingAPI(object): url = self.makeurl(['source', project, package, '_meta']) http_PUT(url, data=dst_meta) - def check_one_request(self, request, project): - """ - Check if a staging request is ready to be approved. Reviews - for the project are ignored, other open reviews will block the - acceptance - :param project: staging project - :param request_id: request id to check - - """ - - url = self.makeurl(['request', str(request)]) - root = ET.parse(http_GET(url)).getroot() - - # relevant info for printing - package = str(root.find('action').find('target').attrib['package']) - - state = root.find('state').get('name') - if state in ['declined', 'superseded', 'revoked']: - return '{}: {}'.format(package, state) - - # instead of just printing the state of the whole request find - # out who is remaining on the review and print it out, - # otherwise print out that it is ready for approval and - # waiting on others from GR to be accepted - review_state = root.findall('review') - failing_groups = [] - for i in review_state: - if i.attrib['state'] == 'accepted': - continue - if i.get('by_project', None) == project: - continue - for attrib in ['by_group', 'by_user', 'by_project', 'by_package']: - value = i.get(attrib, None) - if value: - failing_groups.append(value) - - if not failing_groups: - return None - else: - state = 'missing reviews: ' + ', '.join(failing_groups) - return '{}: {}'.format(package, state) - def check_ring_packages(self, project, requests): """ Checks if packages from requests are in some ring or not @@ -556,88 +513,21 @@ class StagingAPI(object): return False - def check_project_status(self, project, verbose=False): + def check_project_status(self, project): """ - Checks a staging project for acceptance. Checks all open - requests for open reviews and build status + Checks a staging project for acceptance. Use the JSON document + for staging project to base the decision. :param project: project to check :return true (ok)/false (empty prj) or list of strings with informations) + """ - - # Report - report = list() - - # First ensure we dispatched the open requests so we do not - # pass projects with update/superseded requests - for request in self.get_open_requests(): - stage_info = self.supseded_request(request) - if stage_info and stage_info['prj'] == project: - return ['Request {} is superseded'.format(stage_info['rq_id'])] - - # all requests with open review - requests = self.list_requests_in_prj(project) - open_requests = set(requests) - - # all tracked requests - some of them might be declined, so we - # don't see them above - meta = self.get_prj_pseudometa(project) - for req in meta['requests']: - req = req['id'] - if req in open_requests: - open_requests.remove(req) - if req not in requests: - requests.append(req) - if open_requests: - return ['Request(s) {} are not tracked but are open for the prj'.format(','.join(map(str, open_requests)))] - - # If we find no requests in staging then it is empty so we ignore it - if not requests: - return False - - # Check if the requests are acceptable and bail out on - # first failure unless verbose as it is slow - for request in requests: - ret = self.check_one_request(request, project) - if ret: - report.append(ret) - if not verbose: - break - - # Check the build/openQA only if we have some ring packages - if self.check_ring_packages(project, requests): - # Check the buildstatus - buildstatus = self.gather_build_status(project) - if buildstatus: - # here no append as we are adding list to list - report += self.generate_build_status_details(buildstatus, verbose) - # Check the openqa state - ret = self.find_openqa_state(project) - if ret: - report.append(ret) - - if report: - return report - elif not self.project_exists(project + ":DVD"): - # The only case we are green - return True - - # now check the same for the subprj - project = project + ":DVD" - buildstatus = self.gather_build_status(project) - - if buildstatus: - report += self.generate_build_status_details(buildstatus, verbose) - - # Check the openqa state - ret = self.find_openqa_state(project) - if ret: - report.append(ret) - - if report: - return report - - return True + _prefix = 'openSUSE:Factory:Staging:' + if project.startswith(_prefix): + project = project.replace(_prefix, '') + url = self.makeurl(('factory', 'staging_projects', project + '.json')) + result = json.load(self.retried_GET(url)) + return result['overall_state'] == 'acceptable' def days_since_last_freeze(self, project): """ @@ -652,66 +542,6 @@ class StagingAPI(object): return (time.time() - float(entry.get('mtime')))/3600/24 return 100000 # quite some! - def find_openqa_jobs(self, project): - url = self.makeurl(['build', project, 'images', 'x86_64', 'Test-DVD-x86_64']) - root = ET.parse(http_GET(url)).getroot() - - filename = None - for binary in root.findall('binary'): - filename = binary.get('filename', '') - if filename.endswith('.iso'): - break - - if not filename: - return None - - jobtemplate = '-Staging' - if project.endswith(':DVD'): - jobtemplate = '-Staging2' - project = project[:-4] - - jobname = 'openSUSE-Staging:' - jobname += project.split(':')[-1] + jobtemplate - result = re.match('Test-Build([^-]+)-Media.iso', filename) - jobname += '-DVD-x86_64-Build' + result.group(1) + "-Media.iso" - - try: - url = "https://openqa.opensuse.org/api/v1/jobs?iso={}".format(jobname) - f = urllib2.urlopen(url) - except urllib2.HTTPError: - return None - - jobs = json.load(f)['jobs'] - - bestjobs = {} - for job in jobs: - if job['result'] != 'incomplete' and not job['clone_id']: - if job['test'] == 'miniuefi': - continue - if job['name'] not in bestjobs or bestjobs[job['name']]['result'] != 'passed': - bestjobs[job['name']] = job - - return bestjobs - - def find_openqa_state(self, project): - """ - Checks the openqa state of the project - :param project: project to check - :return None or list with issue informations - """ - - jobs = self.find_openqa_jobs(project) - - if not jobs: - return 'No openQA result yet' - - for job in jobs.values(): - check = self.check_if_job_is_ok(job) - if check: - return check - - return None - def check_if_job_is_ok(self, job): url = 'https://openqa.opensuse.org/tests/{}/file/results.json'.format(job['id']) try: @@ -741,72 +571,6 @@ class StagingAPI(object): return '{} test failed: https://openqa.opensuse.org/tests/{}'.format(module['name'], job['id']) return None - def gather_build_status(self, project): - """ - Checks whether everything is built in project - :param project: project to check - """ - # Get build results - url = self.makeurl(['build', project, '_result?code=failed&code=broken&code=unresolvable']) - root = ET.parse(http_GET(url)).getroot() - - # Check them - broken = [] - working = [] - # Iterate through repositories - for results in root.findall('result'): - building = False - if results.get('state') not in ('published', 'unpublished') or results.get('dirty') == 'true': - working.append({ - 'path': '{}/{}'.format(results.get('repository'), - results.get('arch')), - 'state': results.get('state') - }) - building = True - # Iterate through packages - for node in results: - # Find broken - result = node.get('code') - if result in ('broken', 'failed') or (result == 'unresolvable' and not building): - broken.append({ - 'pkg': node.get('package'), - 'state': result, - 'path': '{}/{}'.format(results.get('repository'), - results.get('arch')) - }) - - # Print the results - if not working and not broken: - return None - else: - return [project, working, broken] - - def generate_build_status_details(self, details, verbose=False): - """ - Generate list of strings for the buildstatus detail report. - :param details: buildstatus informations about project - :return list of strings for printing - """ - - retval = list() - if not isinstance(details, list): - return retval - project, working, broken = details - - if working: - retval.append('At least following repositories is still building:') - for i in working: - retval.append(' {}: {}'.format(i['path'], i['state'])) - if not verbose: - break - if broken: - retval.append('Following packages are broken:') - for i in broken: - retval.append(' {} ({}): {}'.format(i['pkg'], i['path'], - i['state'])) - - return retval - def rq_to_prj(self, request_id, project): """ Links request to project - delete or submit diff --git a/tests/accept_tests.py b/tests/accept_tests.py index f67c9c36..c41e6ef0 100644 --- a/tests/accept_tests.py +++ b/tests/accept_tests.py @@ -5,7 +5,6 @@ # Distribute under GPLv2 or later import unittest -import mock from obs import APIURL from obs import OBS @@ -29,8 +28,7 @@ class TestAccept(unittest.TestCase): comments = c_api.get_comments(project_name=staging_c) # Accept staging C (containing apparmor and mariadb) - with mock.patch('osclib.stagingapi.StagingAPI.find_openqa_state', return_value='Nothing'): - self.assertEqual(True, AcceptCommand(self.api).perform(staging_c)) + self.assertEqual(True, AcceptCommand(self.api).perform(staging_c)) # Comments are cleared up accepted_comments = c_api.get_comments(project_name=staging_c) diff --git a/tests/api_tests.py b/tests/api_tests.py index c82438e0..dc6ef976 100644 --- a/tests/api_tests.py +++ b/tests/api_tests.py @@ -7,7 +7,6 @@ import sys import unittest import httpretty -import mock from obs import APIURL from obs import OBS @@ -138,34 +137,6 @@ class TestApiCalls(unittest.TestCase): # Get package name self.assertEqual('wine', self.api.get_package_for_request_id(prj, num)) - def test_check_one_request(self): - prj = 'openSUSE:Factory:Staging:B' - pkg = 'wine' - - full_name = prj + '/' + pkg - - # Verify package is there - self.assertTrue(full_name in self.obs.links) - # Get rq number - num = self.api.get_request_id_for_package(prj, pkg) - # Check the results - self.assertEqual(self.api.check_one_request(num, prj), None) - # Pretend to be reviewed by other project - self.assertEqual(self.api.check_one_request(num, 'xyz'), - 'wine: missing reviews: openSUSE:Factory:Staging:B') - - def test_check_project_status(self): - # Check the results - with mock.patch('osclib.stagingapi.StagingAPI.find_openqa_state', return_value='Nothing'): - broken_results = ['At least following repositories is still building:', - ' building/x86_64: building', - 'Following packages are broken:', - ' wine (failed/x86_64): failed', - ' wine (broken/x86_64): broken', - 'Nothing'] - self.assertEqual(self.api.check_project_status('openSUSE:Factory:Staging:B'), broken_results) - self.assertEqual(self.api.check_project_status('openSUSE:Factory:Staging:A'), False) - def test_rm_from_prj(self): prj = 'openSUSE:Factory:Staging:B' pkg = 'wine' @@ -230,26 +201,6 @@ class TestApiCalls(unittest.TestCase): self.assertEqual(self.api.get_prj_pseudometa('openSUSE:Factory:Staging:A'), {'requests': [{'id': 123, 'package': 'gcc', 'author': 'Admin'}]}) - def test_generate_build_status_details(self): - """Check whether generate_build_status_details works.""" - - details_green = self.api.gather_build_status('green') - details_red = self.api.gather_build_status('red') - red = ['red', [{'path': 'standard/x86_64', 'state': 'building'}], - [{'path': 'standard/i586', 'state': 'broken', 'pkg': 'glibc'}, - {'path': 'standard/i586', 'state': 'failed', 'pkg': 'openSUSE-images'}]] - red_result = ['At least following repositories is still building:', - ' standard/x86_64: building', - 'Following packages are broken:', - ' glibc (standard/i586): broken', - ' openSUSE-images (standard/i586): failed'] - - self.assertEqual(details_red, red) - self.assertEqual(self.api.generate_build_status_details(details_red), red_result) - self.assertEqual(self.api.generate_build_status_details(details_red, True), red_result) - self.assertEqual(details_green, None) - self.assertEqual(self.api.generate_build_status_details(details_green), []) - def test_create_package_container(self): """Test if the uploaded _meta is correct.""" @@ -291,22 +242,6 @@ class TestApiCalls(unittest.TestCase): self.assertEqual(self.api.prj_from_letter('openSUSE:Factory'), 'openSUSE:Factory') self.assertEqual(self.api.prj_from_letter('A'), 'openSUSE:Factory:Staging:A') - def test_check_project_status_green(self): - """Test checking project status.""" - - # Check print output - self.assertEqual(self.api.gather_build_status('green'), None) - - def test_check_project_status_red(self): - """Test checking project status.""" - - # Check print output - self.assertEqual( - self.api.gather_build_status('red'), - ['red', [{'path': 'standard/x86_64', 'state': 'building'}], - [{'path': 'standard/i586', 'pkg': 'glibc', 'state': 'broken'}, - {'path': 'standard/i586', 'pkg': 'openSUSE-images', 'state': 'failed'}]]) - def test_frozen_mtime(self): """Test frozen mtime.""" diff --git a/tests/check_tests.py b/tests/check_tests.py index dc59bfc4..80d9caaf 100644 --- a/tests/check_tests.py +++ b/tests/check_tests.py @@ -29,13 +29,13 @@ FULL_REPORT = """ ++ Acceptable staging project openSUSE:Factory:Staging:C - -- Project openSUSE:Factory:Staging:F still needs attention + -- REVIEW Project openSUSE:Factory:Staging:F still needs attention - yast2-iscsi-client: Missing reviews: factory-repo-checker - -- Project openSUSE:Factory:Staging:G still needs attention + -- REVIEW Project openSUSE:Factory:Staging:G still needs attention - Mesa: Missing reviews: opensuse-review-team - -- Project openSUSE:Factory:Staging:H still needs attention + -- BUILDING Project openSUSE:Factory:Staging:H still needs attention - kiwi: Missing reviews: opensuse-review-team - At least following repositories are still building: standard/i586: building @@ -46,12 +46,12 @@ FULL_REPORT = """ - At least following repositories are still building: standard/x86_64: blocked - -- Project openSUSE:Factory:Staging:J still needs attention + -- REVIEW Project openSUSE:Factory:Staging:J still needs attention - jeuclid: Missing reviews: factory-repo-checker """ H_REPORT = """ - -- Project openSUSE:Factory:Staging:H still needs attention + -- BUILDING Project openSUSE:Factory:Staging:H still needs attention - kiwi: Missing reviews: opensuse-review-team - At least following repositories are still building: standard/i586: scheduling diff --git a/tests/fixtures/factory/staging_projects.json b/tests/fixtures/factory/staging_projects.json index 2cab5ba1..3ee0ac03 100644 --- a/tests/fixtures/factory/staging_projects.json +++ b/tests/fixtures/factory/staging_projects.json @@ -1,5 +1,6 @@ [ { + "overall_state": "acceptable", "selected_requests": [ { "id": 235410 @@ -81,6 +82,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -163,6 +165,7 @@ "untracked_requests": [] }, { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -240,6 +243,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -322,6 +326,7 @@ "untracked_requests": [] }, { + "overall_state": "acceptable", "selected_requests": [ { "id": 235517 @@ -403,6 +408,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -485,6 +491,7 @@ "untracked_requests": [] }, { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -562,6 +569,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -644,6 +652,7 @@ "untracked_requests": [] }, { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -721,6 +730,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -803,6 +813,7 @@ "untracked_requests": [] }, { + "overall_state": "review", "selected_requests": [ { "id": 234678 @@ -1006,6 +1017,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -1088,6 +1100,7 @@ "untracked_requests": [] }, { + "overall_state": "review", "selected_requests": [ { "id": 234718 @@ -1216,6 +1229,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -1298,6 +1312,7 @@ "untracked_requests": [] }, { + "overall_state": "building", "selected_requests": [ { "id": 234999 @@ -1421,6 +1436,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [ @@ -1593,6 +1609,7 @@ ], "subprojects": [ { + "overall_state": "empty", "selected_requests": [], "broken_packages": [], "building_repositories": [], @@ -1633,6 +1650,7 @@ "untracked_requests": [] }, { + "overall_state": "review", "selected_requests": [ { "id": 235022 diff --git a/tests/fixtures/factory/staging_projects/C.json b/tests/fixtures/factory/staging_projects/C.json new file mode 100644 index 00000000..430cc4cb --- /dev/null +++ b/tests/fixtures/factory/staging_projects/C.json @@ -0,0 +1,3 @@ +{ + "overall_state": "acceptable" +} diff --git a/tests/fixtures/factory/staging_projects/H.json b/tests/fixtures/factory/staging_projects/H.json index 160105f0..2b62a401 100644 --- a/tests/fixtures/factory/staging_projects/H.json +++ b/tests/fixtures/factory/staging_projects/H.json @@ -1,4 +1,5 @@ { + "overall_state": "building", "broken_packages": [], "building_repositories": [ { @@ -104,6 +105,7 @@ ], "subprojects": [ { + "overall_state": "empty", "broken_packages": [], "building_repositories": [ {