From 76bf2ba5afd3ebc7c98246151f38d25d206d4f26 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Nov 02 2020 14:34:31 +0000 Subject: cli, frontend: custom build batches Users can batch the builds now, preferably using the new copr-cli build options --with-build-id and --after-build-id. There is a new "status" page for batches, so people can watch the progress. --- diff --git a/cli/copr_cli/main.py b/cli/copr_cli/main.py index f1c9108..899c87a 100644 --- a/cli/copr_cli/main.py +++ b/cli/copr_cli/main.py @@ -114,8 +114,10 @@ def buildopts_from_args(args, progress_callback): "background": args.background, "progress_callback": progress_callback, } - if args.bootstrap is not None: - buildopts["bootstrap"] = args.bootstrap + for opt in ["bootstrap", "after_build_id", "with_build_id"]: + value = getattr(args, opt) + if value is not None: + buildopts[opt] = value return buildopts @@ -1155,6 +1157,17 @@ def setup_parser(): "to the pre-configured setup from mock-core-configs. " "See 'create --help' for more info.")) + batch_build_opts = parser_build_parent.add_mutually_exclusive_group() + batch_build_opts.add_argument( + "--after-build-id", metavar="BUILD_ID", + help=("Build after the batch containing the BUILD_ID build."), + ) + + batch_build_opts.add_argument( + "--with-build-id", metavar="BUILD_ID", + help=("Build in the same batch with the BUILD_ID build."), + ) + # create the parser for the "build" (url/upload) command parser_build = subparsers.add_parser("build", parents=[parser_build_parent], help="Build packages to a specified copr") diff --git a/frontend/copr-frontend.spec b/frontend/copr-frontend.spec index 586ce16..15d7648 100644 --- a/frontend/copr-frontend.spec +++ b/frontend/copr-frontend.spec @@ -69,6 +69,7 @@ BuildRequires: python3-devel %if %{with check} BuildRequires: fedora-messaging +BuildRequires: python3-anytree BuildRequires: python3-click BuildRequires: python3-CommonMark BuildRequires: python3-blinker @@ -127,6 +128,7 @@ Requires: fedora-messaging Requires: js-html5shiv Requires: js-jquery Requires: js-respond +Requires: python3-anytree Requires: python3-click Requires: python3-CommonMark Requires: python3-alembic diff --git a/frontend/coprs_frontend/coprs/__init__.py b/frontend/coprs_frontend/coprs/__init__.py index a0db6ef..34b02c0 100644 --- a/frontend/coprs_frontend/coprs/__init__.py +++ b/frontend/coprs_frontend/coprs/__init__.py @@ -82,6 +82,9 @@ from coprs.views import apiv3_ns from coprs.views.apiv3_ns import (apiv3_general, apiv3_builds, apiv3_packages, apiv3_projects, apiv3_project_chroots, apiv3_modules, apiv3_build_chroots, apiv3_mock_chroots, apiv3_permissions) + +from coprs.views import batches_ns +from coprs.views.batches_ns import coprs_batches from coprs.views import coprs_ns from coprs.views.coprs_ns import coprs_builds from coprs.views.coprs_ns import coprs_general @@ -123,6 +126,7 @@ setup_log() app.register_blueprint(api_ns.api_ns) app.register_blueprint(apiv3_ns.apiv3_ns) app.register_blueprint(admin_ns.admin_ns) +app.register_blueprint(batches_ns.batches_ns) app.register_blueprint(coprs_ns.coprs_ns) app.register_blueprint(misc.misc) app.register_blueprint(backend_ns.backend_ns) diff --git a/frontend/coprs_frontend/coprs/forms.py b/frontend/coprs_frontend/coprs/forms.py index 0010e40..bc87517 100644 --- a/frontend/coprs_frontend/coprs/forms.py +++ b/frontend/coprs_frontend/coprs/forms.py @@ -1101,6 +1101,57 @@ class BaseBuildFormFactory(object): F.chroots_sets[ch[0]].append(ch) else: F.chroots_sets[ch[0]] = [ch] + + F.after_build_id = wtforms.IntegerField( + "Batch-build after", + description=( + "Optional - Build after the batch containing " + "the Build ID build." + ), + validators=[ + wtforms.validators.Optional()], + render_kw={'placeholder': 'Build ID'}, + filters=[NoneFilter(None)], + ) + + F.with_build_id = wtforms.IntegerField( + "Batch-build with", + description=( + "Optional - Build in the same batch with the Build ID build" + ), + render_kw={'placeholder': 'Build ID'}, + validators=[ + wtforms.validators.Optional()], + filters=[NoneFilter(None)], + ) + + def _validate_batch_opts(form, field): + counterpart = form.with_build_id + modifies = False + if counterpart == field: + counterpart = form.after_build_id + modifies = True + + if counterpart.data: + raise wtforms.ValidationError( + "Only one batch option can be specified") + + build_id = field.data + if not build_id: + return + + build_id = int(build_id) + build = models.Build.query.get(build_id) + if not build: + raise wtforms.ValidationError( + "Build {} not found".format(build_id)) + batch_error = build.batching_user_error(flask.g.user, modifies) + if batch_error: + raise wtforms.ValidationError(batch_error) + + F.validate_with_build_id = _validate_batch_opts + F.validate_after_build_id = _validate_batch_opts + return F diff --git a/frontend/coprs_frontend/coprs/logic/batches_logic.py b/frontend/coprs_frontend/coprs/logic/batches_logic.py new file mode 100644 index 0000000..25796ff --- /dev/null +++ b/frontend/coprs_frontend/coprs/logic/batches_logic.py @@ -0,0 +1,120 @@ +""" +Methods for working with build Batches. +""" + +import anytree + +from coprs import db +from coprs.helpers import WorkList +from coprs.models import Batch, Build +from coprs.exceptions import BadRequest +import coprs.logic.builds_logic as bl + + +class BatchesLogic: + """ Batch logic entrypoint """ + @classmethod + def get_batch_or_create(cls, build_id, requestor, modify=False): + """ + Put the build into a new batch, and return the batch. If the build is + already assigned to any batch, do nothing and return the batch. + + Locks the build for updates, may block! + """ + + # We don't want to create a new batch if one already exists, but there's + # the concurrency problem so we need to lock the build instance for + # writing. + build = db.session.query(Build).with_for_update().get(build_id) + if not build: + raise BadRequest("Build {} doesn't exist".format(build_id)) + + # Somewhat pedantically, we _should_ lock the batch (if exists) + # here because the query for 'build.finished' and + # 'build.batch.finished' is a bit racy (backend workers may + # asynchronously make the build/batch finished, and we may still + # assign some new build to a just finished batch). + error = build.batching_user_error(requestor, modify) + if error: + raise BadRequest(error) + + if build.batch: + return build.batch + + batch = Batch() + db.session.add(batch) + build.batch = batch + return batch + + @staticmethod + def pending_batches(): + """ + Query for all still not-finished batches, order by id ASC + """ + batches = set() + query = bl.BuildsLogic.processing_builds() + for build in query.all(): + if build.batch: + batches.add(build.batch) + return batches + + @classmethod + def pending_batch_trees(cls): + """ + Get all the currently processing batches, together with all the + dependency batches which are already finished -- and keep them ordered + in list based on theirs ID and dependencies. + """ + roots = [] + node_map = {} + def get_mapped_node(batch): + if batch.id in node_map: + return node_map[batch.id] + node_map[batch.id] = anytree.Node(batch) + return node_map[batch.id] + + # go through all the batches transitively + pending_batches = cls.pending_batches() + wl = WorkList(pending_batches) + while not wl.empty: + batch = wl.pop() + node = get_mapped_node(batch) + if batch.blocked_by_id: + parent_node = get_mapped_node(batch.blocked_by) + node.parent = parent_node + wl.schedule(batch.blocked_by) + else: + roots.append(node) + return roots + + @classmethod + def batch_chain(cls, batch_id): + """ + Return the batch_with batch_id, and all the transitively blocking + batches in one list. + """ + chain = [] + batch = Batch.query.get(batch_id) + while batch: + chain.append(batch) + batch = batch.blocked_by + return chain + + # STILL PENDING + # ============= + # => some builds are: waiting, pending, starting, running, importing + # => the rest is: succeeded/failed + # + # SUCCEEDED + # ========= + # => all builds succeeded + # + # FAILED, BUT FIXABLE + # =================== + # => all builds are succeeded or failed + # => timeout is OK: last ended_on is >= time.time() - deadline + # + # FAILED + # ====== + # => some builds failed + # => timeout is out diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py index 4da0760..daac940 100644 --- a/frontend/coprs_frontend/coprs/logic/builds_logic.py +++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py @@ -9,7 +9,7 @@ import requests from sqlalchemy.sql import text from sqlalchemy.sql.expression import not_ from sqlalchemy.orm import joinedload, selectinload -from sqlalchemy import func, desc +from sqlalchemy import func, desc, or_, and_ from sqlalchemy.sql import false,true from werkzeug.utils import secure_filename from sqlalchemy import bindparam, Integer, String @@ -37,13 +37,19 @@ from coprs.logic import users_logic from coprs.logic.actions_logic import ActionsLogic from coprs.logic.dist_git_logic import DistGitLogic from coprs.models import BuildChroot -from .coprs_logic import MockChrootsLogic +from coprs.logic.coprs_logic import MockChrootsLogic from coprs.logic.packages_logic import PackagesLogic +from coprs.logic.batches_logic import BatchesLogic from .helpers import get_graph_parameters log = app.logger +PROCESSING_STATES = [StatusEnum(s) for s in [ + "running", "pending", "starting", "importing", "waiting", +]] + + class BuildsLogic(object): @classmethod def get(cls, build_id): @@ -600,6 +606,8 @@ class BuildsLogic(object): srpm_url=srpm_url, copr_dirname=copr_dirname, bootstrap=build_options.get("bootstrap"), + after_build_id=build_options.get("after_build_id"), + with_build_id=build_options.get("with_build_id"), ) if "timeout" in build_options: @@ -608,11 +616,29 @@ class BuildsLogic(object): return build @classmethod + def _setup_batch(cls, batch, after_build_id, with_build_id, user): + # those three are exclusive! + if sum([bool(x) for x in + [batch, with_build_id, after_build_id]]) > 1: + raise BadRequest("Multiple build batch specifiers") + + if with_build_id: + batch = BatchesLogic.get_batch_or_create(with_build_id, user, True) + + if after_build_id: + old_batch = BatchesLogic.get_batch_or_create(after_build_id, user) + batch = models.Batch() + batch.blocked_by = old_batch + db.session.add(batch) + + return batch + + @classmethod def add(cls, user, pkgs, copr, source_type=None, source_json=None, repos=None, chroots=None, timeout=None, enable_net=True, git_hashes=None, skip_import=False, background=False, batch=None, srpm_url=None, copr_dirname=None, bootstrap=None, - package=None): + package=None, after_build_id=None, with_build_id=None): if chroots is None: chroots = [] @@ -623,6 +649,8 @@ class BuildsLogic(object): user, copr, "You don't have permissions to build in this copr.") + batch = cls._setup_batch(batch, after_build_id, with_build_id, user) + if not repos: repos = copr.repos @@ -1143,6 +1171,26 @@ class BuildsLogic(object): if counter > 0: db.session.commit() + @classmethod + def processing_builds(cls): + """ + Query for all the builds which are not yet finished, it means all the + builds that have non-finished source status, or any non-finished + existing build chroot. + """ + build_ids_with_bch = db.session.query(BuildChroot.build_id).filter( + BuildChroot.status.in_(PROCESSING_STATES), + ) + # skip waiting state, we need to fix issue #1539 + source_states = set(PROCESSING_STATES)-{StatusEnum("waiting")} + return models.Build.query.filter(and_( + not_(models.Build.canceled), + or_( + models.Build.id.in_(build_ids_with_bch), + models.Build.source_status.in_(source_states), + ), + )) + class BuildChrootsLogic(object): @classmethod diff --git a/frontend/coprs_frontend/coprs/logic/complex_logic.py b/frontend/coprs_frontend/coprs/logic/complex_logic.py index f8802f3..545be3e 100644 --- a/frontend/coprs_frontend/coprs/logic/complex_logic.py +++ b/frontend/coprs_frontend/coprs/logic/complex_logic.py @@ -12,6 +12,7 @@ from coprs import models from coprs import exceptions from coprs.exceptions import ObjectNotFound, ActionInProgressException from coprs.logic.builds_logic import BuildsLogic +from coprs.logic.batches_logic import BatchesLogic from coprs.logic.packages_logic import PackagesLogic from coprs.logic.actions_logic import ActionsLogic @@ -265,6 +266,7 @@ class ComplexLogic(object): pending=pending, running=running, starting=starting, + batches=len(BatchesLogic.pending_batches()), ) @classmethod diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py index 88cc54d..031688d 100644 --- a/frontend/coprs_frontend/coprs/models.py +++ b/frontend/coprs_frontend/coprs/models.py @@ -1323,6 +1323,49 @@ class Build(db.Model, helpers.Serializer): return False return self.bootstrap != "unchanged" + def batching_user_error(self, user, modify=False): + """ + Check if the USER can operate with this build in batches, eg create a + new batch for it, or add other builds to the existing batch. Return the + error message (or None, if everything is OK). + """ + # pylint: disable=too-many-return-statements + if self.batch: + if not modify: + # Anyone can create a new batch which **depends on** an already + # existing batch (even if it is owned by someone else) + return None + + if self.batch.finished: + return "Batch {} is already finished".format(self.batch.id) + + if self.batch.can_assign_builds(user): + # user can modify an existing project... + return None + + project_names = [c.full_name for c in self.batch.assigned_projects] + projects = helpers.pluralize("project", project_names) + return ( + "The batch {} belongs to {}. You are not allowed to " + "build there, so you neither can edit the batch." + ).format(self.batch.id, projects) + + # a new batch is needed ... + msgbase = "Build {} is not yet in any batch, and ".format(self.id) + if not user.can_build_in(self.copr): + return msgbase + ( + "user '{}' doesn't have the build permissions in project '{}' " + "to create a new one" + ).format(user.username, self.copr.full_name) + + if self.finished: + return msgbase + ( + "new batch can not be created because the build has " + "already finished" + ) + + return None # new batch can be safely created + class DistGitBranch(db.Model, helpers.Serializer): """ @@ -1826,8 +1869,43 @@ class Batch(db.Model): @property def finished(self): + if not self.builds: + # no builds assigned to this batch (yet) + return False return all([b.finished for b in self.builds]) + @property + def state(self): + if self.blocked_by and not self.blocked_by.finished: + return "blocked" + return "finished" if self.finished else "processing" + + @property + def assigned_projects(self): + """ Get a list (generator) of assigned projects """ + seen = set() + for build in self.builds: + copr = build.copr + if copr in seen: + continue + seen.add(copr) + yield copr + + def can_assign_builds(self, user): + """ + Check if USER has permissions to assign builds to this batch. Since we + support cross-project batches, user is allowed to add a build to this + batch as long as: + - the batch has no builds yet (user has created a new batch now) + - the batch has at least one build which belongs to project where the + user has build access + """ + if not self.builds: + return True + for copr in self.assigned_projects: + if user.can_build_in(copr): + return True + return False class Module(db.Model, helpers.Serializer): id = db.Column(db.Integer, primary_key=True) diff --git a/frontend/coprs_frontend/coprs/static/css/custom-styles.css b/frontend/coprs_frontend/coprs/static/css/custom-styles.css index d13b6f5..045980b 100644 --- a/frontend/coprs_frontend/coprs/static/css/custom-styles.css +++ b/frontend/coprs_frontend/coprs/static/css/custom-styles.css @@ -39,6 +39,10 @@ input.short-input-field { width: 6em; } +input.input-field-12em { + width: 12em; +} + .table_footer { padding: 7px 0 0 0; background: #f5f5f5; diff --git a/frontend/coprs_frontend/coprs/templates/_helpers.html b/frontend/coprs_frontend/coprs/templates/_helpers.html index 55edfa7..1ad5a8f 100644 --- a/frontend/coprs_frontend/coprs/templates/_helpers.html +++ b/frontend/coprs_frontend/coprs/templates/_helpers.html @@ -1,11 +1,11 @@ -{% macro render_field(field, label=None, class=None, info=None) %} +{% macro render_field(field, label=None, class=None, info=None, width=None) %} {% if not kwargs['hidden'] %}
-
+
{{ field(class="form-control" + (" " + class if class else ""), **kwargs)|safe }}
    {% if info %} @@ -313,6 +313,10 @@

    Task Queue

+ + {{ tasks_info.batches }} + Build Batches + {{ tasks_info.importing}} Importing @@ -606,6 +610,8 @@ https://admin.fedoraproject.org/accounts/group/view/{{name}} {{ render_field(form.bootstrap, placeholder='default') }} + {{ render_field(form.with_build_id, class="input-field-12em") }} + {{ render_field(form.after_build_id, class="input-field-12em") }}
    +
  1. + Home +
    2. +
  2. + Batches +
    3. +
  3. + Batch {{ batch.id }} +
    4. +
+{% endblock %} +{% block body %} + +{% macro batch_print(batch) %} + +Batch {{ batch.id }} + +({{ batch.state }}) +{% endmacro %} + +

Batch {{ batch.id }} detail ({{ batch.state }})

+ +{% for dep in deps %} +{% if loop.first %} +

Depends on: +{% else %} + +{% endif %} +{{ batch_print(dep) }} +{% if loop.last %}

{% endif %} +{% endfor %} + +{{ builds_table(batch.builds) }} + +{% endblock %} diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html index 63b0d82..953334f 100644 --- a/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html +++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/_builds_table.html @@ -17,7 +17,7 @@ Build Time Status - {% if g.user and g.user.can_edit(copr) %} + {% if copr and g.user and g.user.can_edit(copr) %} Mark all {% endif %} @@ -52,7 +52,7 @@ {{ build_state(build) }} - {% if g.user and g.user.can_edit(copr) %} + {% if copr and g.user and g.user.can_edit(copr) %} diff --git a/frontend/coprs_frontend/coprs/templates/status.html b/frontend/coprs_frontend/coprs/templates/status.html index 8c59ada..7f8edd3 100644 --- a/frontend/coprs_frontend/coprs/templates/status.html +++ b/frontend/coprs_frontend/coprs/templates/status.html @@ -19,7 +19,7 @@ {% block body %}

Task queue

    - {% for state in ["importing", "pending", "starting", "running"] %} + {% for state in ["importing", "pending", "starting", "running", "batches"] %}
  • {{ state|capitalize }} diff --git a/frontend/coprs_frontend/coprs/templates/status/batch_list.html b/frontend/coprs_frontend/coprs/templates/status/batch_list.html new file mode 100644 index 0000000..8e5eb87 --- /dev/null +++ b/frontend/coprs_frontend/coprs/templates/status/batch_list.html @@ -0,0 +1,48 @@ +{% macro render_tree(node, depth=0, parent=None) %} +{% set batch = node.name %} + + + + Batch {{ node.name.id }} + + + {{ node.name.state }} + + +{% for child in node.children|sort(attribute='name.id', reverse=True) %} +{{ render_tree(child, depth+1, parent=batch.id) }} +{% endfor %} +{% endmacro %} + +{% set state_of_tasks = "batches" %} +{% extends "status.html" %} +{% block title %}List of processing batches{% endblock %} +{% block header %}List of processing batches{% endblock %} +{% block status_breadcrumb %} +
  • Batches
    • +{%endblock%} +{% block status_body %} +

    Currently {{ queue_sizes["batches"] }} active build batches

    +{% for root_node in batch_trees|sort(attribute='name.id', reverse=True) %} +{% if loop.first %} +
    + + + + + + +{% endif %} +{{ render_tree(root_node) }} +{% if loop.last %} + +
    Batch IDBatch state
    +
    + +{% endif %} +{% else %} +

    No batches being currently processed.

    +{% endfor %} +{% endblock %} diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py index 7c3cd29..0f0e986 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py @@ -278,6 +278,8 @@ def process_creating_new_build(copr, form, create_new_build): 'copr_dirname': form.project_dirname.data, 'timeout': form.timeout.data, 'bootstrap': form.bootstrap.data, + 'after_build_id': form.after_build_id.data, + 'with_build_id': form.with_build_id.data, } # From URLs it can be created multiple builds at once diff --git a/frontend/coprs_frontend/coprs/views/batches_ns/__init__.py b/frontend/coprs_frontend/coprs/views/batches_ns/__init__.py new file mode 100644 index 0000000..e046f52 --- /dev/null +++ b/frontend/coprs_frontend/coprs/views/batches_ns/__init__.py @@ -0,0 +1,7 @@ +""" +Plug-in the /batches/ namespace +""" + +import flask + +batches_ns = flask.Blueprint("batches_ns", __name__, url_prefix="/batches") diff --git a/frontend/coprs_frontend/coprs/views/batches_ns/coprs_batches.py b/frontend/coprs_frontend/coprs/views/batches_ns/coprs_batches.py new file mode 100644 index 0000000..14fbea6 --- /dev/null +++ b/frontend/coprs_frontend/coprs/views/batches_ns/coprs_batches.py @@ -0,0 +1,16 @@ +""" +Web-UI routes related to build batches +""" + +from flask import render_template +from coprs.logic.batches_logic import BatchesLogic +from coprs.views.batches_ns import batches_ns + + +@batches_ns.route("/detail//") +def coprs_batch_detail(batch_id): + """ Print the list (tree) of batches """ + chain = BatchesLogic.batch_chain(batch_id) + batch = chain[0] + deps = chain[1:] + return render_template("batches/detail.html", batch=batch, deps=deps) diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py index 44e324e..81b0512 100644 --- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -120,6 +120,8 @@ def process_new_build(copr, form, create_new_build_factory, add_function, add_vi "enable_net": form.enable_net.data, "timeout": form.timeout.data, "bootstrap": form.bootstrap.data, + "with_build_id": form.with_build_id.data, + "after_build_id": form.after_build_id.data, } try: diff --git a/frontend/coprs_frontend/coprs/views/status_ns/status_general.py b/frontend/coprs_frontend/coprs/views/status_ns/status_general.py index 8cf5417..957c813 100644 --- a/frontend/coprs_frontend/coprs/views/status_ns/status_general.py +++ b/frontend/coprs_frontend/coprs/views/status_ns/status_general.py @@ -3,6 +3,7 @@ from time import time from copr_common.enums import StatusEnum from coprs.views.status_ns import status_ns +from coprs.logic import batches_logic from coprs.logic import builds_logic from coprs.logic import complex_logic @@ -73,6 +74,22 @@ def render_status(build_status, tasks, bg_tasks_cnt=None): state_of_tasks=build_status) +@status_ns.route("/batches/") +def batches(): + """ Print the list (tree) of batches """ + trees = batches_logic.BatchesLogic.pending_batch_trees() + return flask.render_template("status/batch_list.html", batch_trees=trees) + + +@status_ns.route("/batches/detail//") +def coprs_batch_detail(batch_id): + """ Print the list (tree) of batches """ + chain = batches_logic.BatchesLogic.batch_chain(batch_id) + batch = chain[0] + deps = chain[1:] + return flask.render_template("batches/detail.html", batch=batch, deps=deps) + + @status_ns.route("/stats/") def stats(): curr_time = int(time()) diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py index c1ed81d..8796e6e 100644 --- a/frontend/coprs_frontend/tests/coprs_test_case.py +++ b/frontend/coprs_frontend/tests/coprs_test_case.py @@ -1,5 +1,6 @@ # pylint: disable=attribute-defined-outside-init import base64 +from contextlib import contextmanager import json import os import time @@ -26,7 +27,6 @@ class CoprsTestCase(object): # These are made available by TransactionDecorator() decorator test_client = None - transaction_user = None transaction_username = None original_config = coprs.app.config.copy() @@ -749,6 +749,34 @@ def foo(): for bch in self.b4_bc: bch.status = StatusEnum("succeeded") + @contextmanager + def setup_user_session(self, user): + """ + Setup session cookie, according to Flask docs: + https://flask.palletsprojects.com/en/1.1.x/testing/#accessing-and-modifying-sessions + """ + self.transaction_user = user + self.transaction_username = user.username + with self.tc as self.test_client: + # create one fake request that will setup the session value, + # and keep the value in the next requests ... + with self.test_client.session_transaction() as session: + session["openid"] = user.username + # ... as long as the self.test_client variable lives + yield + + @pytest.fixture + def f_u1_ts_client(self, f_users, f_users_api, f_db): + """ + This is alternative for the per-test @TransactionDecorator("u1") + decorator. And can be used also on a per-class level. Note that + we need to commit the session, otherwise the user is unknown to + the client. + """ + _just_fixtures = f_users, f_users_api, f_db + with self.setup_user_session(self.u1): + yield + def request_rest_api_with_auth(self, url, login=None, token=None, content=None, method="GET", @@ -849,11 +877,7 @@ class TransactionDecorator(object): @wraps(fn) def wrapper(fn, fn_self, *args): user = getattr(fn_self, self.user) - fn_self.transaction_user = user - fn_self.transaction_username = user.username - with fn_self.tc as fn_self.test_client: - with fn_self.test_client.session_transaction() as session: - session["openid"] = user.username + with fn_self.setup_user_session(user): return fn(fn_self, *args) return decorator.decorator(wrapper, fn) diff --git a/frontend/coprs_frontend/tests/request_test_api.py b/frontend/coprs_frontend/tests/request_test_api.py index d14baaf..1a49302 100644 --- a/frontend/coprs_frontend/tests/request_test_api.py +++ b/frontend/coprs_frontend/tests/request_test_api.py @@ -124,7 +124,7 @@ class WebUIRequests(_RequestsInterface): for ch in chroots: form_data[ch] = 'y' - for attr in ["bootstrap"]: + for attr in ["bootstrap", "with_build_id", "after_build_id"]: value = build_options.get(attr) if value is None: continue @@ -214,7 +214,7 @@ class API3Requests(_RequestsInterface): if not build_options: build_options = {} form_data = {} - for arg in ["chroots", "bootstrap"]: + for arg in ["chroots", "bootstrap", "with_build_id", "after_build_id"]: if arg not in build_options: continue if build_options[arg] is None: diff --git a/frontend/coprs_frontend/tests/test_logic/test_batch_logic.py b/frontend/coprs_frontend/tests/test_logic/test_batch_logic.py new file mode 100644 index 0000000..069c22e --- /dev/null +++ b/frontend/coprs_frontend/tests/test_logic/test_batch_logic.py @@ -0,0 +1,104 @@ +""" +Tests for working with Batches +""" + +import pytest +from copr_common.enums import StatusEnum +from coprs import models +from coprs.exceptions import BadRequest +from coprs.logic.batches_logic import BatchesLogic +from tests.coprs_test_case import CoprsTestCase + + +@pytest.mark.usefixtures("f_u1_ts_client", "f_mock_chroots", "f_db") +class TestBatchesLogic(CoprsTestCase): + batches = None + + def _prepare_project_with_batches(self): + self.web_ui.new_project("test", ["fedora-rawhide-i386"]) + assert models.Copr.query.count() == 1 + self.api3.submit_url_build("test") + self.web_ui.submit_url_build("test", build_options={ + "after_build_id": 1, + }) + self.api3.submit_url_build("test", build_options={ + "with_build_id": 1, + }) + self.web_ui.submit_url_build("test", build_options={ + "with_build_id": 2, + }) + self.batches = batches = models.Batch.query.all() + assert len(batches) == 2 + for batch in batches: + assert len(batch.builds) == 2 + + def _succeed_first_batch(self): + for build in self.batches[0].builds: + build.source_status = StatusEnum("succeeded") + for chroot in build.build_chroots: + chroot.state = StatusEnum("succeeded") + assert build.finished + assert self.batches[0].finished + self.db.session.commit() + + def _submit(self, build_options, status=400): + resp = self.api3.submit_url_build("test", build_options=build_options) + assert resp.status_code == status + return resp.data.decode("utf-8") + + def test_normal_batch_operation_failures(self): + self._prepare_project_with_batches() + self._succeed_first_batch() + + # we can not assign builds to finished builds + error = self._submit({"with_build_id": 1}) + assert "Batch 1 is already finished" in error + + # we can not assign builds to non-existing builds, and try spaces + # around numbers, too + error = self._submit({"with_build_id": ' 6 '}) + assert "Build 6 not found" in error + + # both batch options + error = self._submit({"with_build_id": 1, "after_build_id": 2}) + assert "Only one batch option" in error + + # invalid int + error = self._submit({"with_build_id": "None"}) + assert "Not a valid integer" in error + + # drop the finished build from batch + build = models.Build.query.get(1) + build.batch = None + self.db.session.commit() + error = self._submit({"with_build_id": 1}) + assert "new batch can not be created" in error + assert "already finished" in error + + def test_less_likely_batch_problems(self): + self._prepare_project_with_batches() + # non existing build + with pytest.raises(BadRequest) as error: + BatchesLogic.get_batch_or_create(7, self.transaction_user) + assert "doesn't exist" in str(error) + # existing build + BatchesLogic.get_batch_or_create(2, self.transaction_user) + # permission problem + user = models.User.query.get(2) + with pytest.raises(BadRequest) as error: + BatchesLogic.get_batch_or_create(2, user, modify=True) + assert "The batch 2 belongs to project user1/test" in str(error) + assert "You are not allowed to build there" in str(error) + + def test_cant_group_others_build(self): + self._prepare_project_with_batches() + # de-assign the build from batch + build = models.Build.query.get(1) + build.batch = None + self.db.session.commit() + + user = models.User.query.get(2) + with pytest.raises(BadRequest) as error: + BatchesLogic.get_batch_or_create(1, user, modify=True) + assert "Build 1 is not yet in any batch" in str(error) + assert "'user2' doesn't have the build permissions" in str(error)