Syncing from upstream odoo/runbot (19.0-imrove-error-tags-edition-xdo)

runbot/common.py

@@ -329,18 +329,43 @@ class TestTagsParser:
                                 (?:\[(.*)\])?               # parameters
                                 $''', re.VERBOSE)  # [-][tag][/module][:class][.method][[params]]
 
-    def __init__(self, test_tags):
-        parts = re.split(r',(?![^\[]*\])', test_tags)  # split on all comma not inside [] (not followed by ])
+    def __init__(self, test_tags, keep_escape=True):
+        parts = ['']
+        bracket_level = 0
+        escape_next = False
+        for char in test_tags:
+            if char == ',' and bracket_level == 0:
+                parts.append('')
+                continue
+
+            if char == '\\':
+                if not escape_next:
+                    escape_next = True
+                    if keep_escape:
+                        parts[-1] += '\\'  # not as the TagsSelector, we keep the escape character
+                    continue
+            elif char == '[':
+                if not escape_next:
+                    bracket_level += 1
+            elif char == ']':
+                if not escape_next:
+                    bracket_level -= 1
+            elif not keep_escape and escape_next:  # the previous \ was not escaping anything, put it back
+                parts[-1] += '\\'
+
+            escape_next = False
+            parts[-1] += char
+
         filter_specs = [t.strip() for t in parts if t.strip()]
+        self.filter_specs = filter_specs
         self.exclude = set()
         self.include = set()
         self.parameters = OrderedSet()
 
         for filter_spec in filter_specs:
             match = self.filter_spec_re.match(filter_spec)
             if not match:
-                _logger.error('Invalid tag %s', filter_spec)
-                continue
+                raise ValueError('Invalid tag %s' % filter_spec)
 
             sign, tag, file_path, module, klass, method, parameters = match.groups()
             is_include = sign != '-'
@@ -369,13 +394,18 @@ def __init__(self, test_tags):
 
     def test_tags_to_search_domain(self, exclude_error_id=None):
         search_domains = []
+        params_by_spec = dict(self.parameters)
         for include in self.include:
             _, test_module, test_class, test_method, file_path = include
             module_path = file_path or ((test_module or '') + '%')
             test_class = test_class or '%'
             test_method = test_method or '%'
             search_pattern = f'{module_path}:{test_class}.{test_method}'
             tag_domain = [('canonical_tags', 'like', f'{search_pattern}')]
+            params = params_by_spec.get(include)
+            if params:
+                _sign, parameters = params
+                tag_domain.append(('canonical_tags', 'like', f'%[{parameters}%]%'))
             if exclude_error_id:
                 tag_domain.append(('id', '!=', exclude_error_id))
             search_domains.append(tag_domain)

runbot/models/build_config.py

@@ -1,35 +1,46 @@
 import base64
+import fnmatch
 import glob
 import json
 import logging
-import fnmatch
-import psutil
 import re
 import shlex
 import time
-from unidiff import PatchSet
-from ..common import now, grep, time2str, rfind, s2human, os, RunbotException, ReProxy, markdown_escape
-from ..container import docker_get_gateway_ip, Command
-from odoo import models, fields, api, tools
-from odoo.exceptions import UserError, ValidationError
-from odoo.tools.misc import file_open
-from odoo.tools.safe_eval import safe_eval, test_python_expr, _SAFE_OPCODES, to_opcodes
 
-# adding some additionnal optcode to safe_eval. This is not 100% needed and won't be done in standard but will help
-# to simplify some python step by wraping the content in a function to allow return statement and get closer to other
-# steps
+import psutil
+from unidiff import VERSION, PatchSet, patch
 
+from odoo import api, fields, models, tools
+from odoo.exceptions import UserError, ValidationError
+from odoo.tools.misc import file_open
+from odoo.tools.safe_eval import _SAFE_OPCODES, safe_eval, test_python_expr, to_opcodes
+
+from ..common import (
+    ReProxy,
+    RunbotException,
+    TestTagsParser,
+    grep,
+    markdown_escape,
+    now,
+    os,
+    rfind,
+    s2human,
+    time2str,
+)
+from ..container import Command, docker_get_gateway_ip
 
 # There is an issue in unidiff 0.7.3 fixed in 0.7.4
 # https://github.com/matiasb/python-unidiff/commit/a3faffc54e5aacaee3ded4565c534482d5cc3465
 # Since the unidiff packaged version in noble is 0.7.3
 # patching it looks like the easiest solution
 
-from unidiff import patch, VERSION
 if VERSION == '0.7.3':
     patch.RE_DIFF_GIT_DELETED_FILE = re.compile(r'^deleted file mode \d+$')
     patch.RE_DIFF_GIT_NEW_FILE = re.compile(r'^new file mode \d+$')
 
+# adding some additionnal optcode to safe_eval. This is not 100% needed and won't be done in standard but will help
+# to simplify some python step by wraping the content in a function to allow return statement and get closer to other
+# steps
 
 _SAFE_OPCODES |= set(to_opcodes(['LOAD_DEREF', 'STORE_DEREF', 'LOAD_CLOSURE', 'MAKE_CELL', 'COPY_FREE_VARS']))
 
@@ -426,7 +437,7 @@ class ConfigStep(models.Model):
     paths_to_omit = fields.Char('Paths to omit from coverage', tracking=True)
     flamegraph = fields.Boolean('Allow Flamegraph', default=False, tracking=True)
     test_enable = fields.Boolean('Test enable', default=True, tracking=True)
-    test_tags = fields.Char('Test tags', help="comma separated list of test tags", tracking=True)
+    test_tags = fields.Char('Test tags', help="new line (or comma) separated list of test tags", tracking=True)
     enable_auto_tags = fields.Boolean('Allow auto tag', default=True, tracking=True)
     sub_command = fields.Char('Subcommand', tracking=True)
     extra_params = fields.Char('Extra cmd args', tracking=True)
@@ -637,6 +648,7 @@ def _make_python_ctx(self, build):
             'json_loads': json.loads,
             'PatchSet': PatchSet,
             'markdown_escape': markdown_escape,
+            'TestTagsParser': TestTagsParser,
         }
 
     def _run_python(self, build, force=False):
@@ -774,7 +786,7 @@ def _run_install_odoo(self, build, config_data=None):
         test_tags_in_extra = '--test-tags' in extra_params
 
         if (test_enable or test_tags) and "--test-tags" in available_options and not test_tags_in_extra:
-            test_tags = [t.strip() for t in (test_tags or '').split(',')]
+            test_tags = [t.strip() for t in TestTagsParser(test_tags or '').filter_specs]
             if enable_auto_tags and not config_data.get('disable_auto_tags', False):
                 if grep(config_path, "[/module][:class]"):
                     auto_tags = self.env['runbot.build.error']._disabling_tags(build)

runbot/models/build_error.py

@@ -9,16 +9,15 @@
 from dateutil import rrule
 from dateutil.relativedelta import relativedelta
 from markupsafe import Markup
-
 from werkzeug.urls import url_join
 
 from odoo import api, fields, models
 from odoo.exceptions import AccessError, UserError, ValidationError
-from odoo.tools import SQL, lazy, ormcache
 from odoo.fields import Domain
+from odoo.tools import SQL, lazy, ormcache
 
+from ..common import TestTagsParser, transactioncache
 from ..fields import JsonDictField
-from ..common import transactioncache, TestTagsParser
 
 _logger = logging.getLogger(__name__)
 
@@ -242,8 +241,8 @@ class BuildError(models.Model):
     breaking_bundle_url = fields.Char('Breaking bundle url', related='breaking_bundle_id.frontend_url')
     breaking_pr_date = fields.Datetime('Breaking date', related="breaking_pr_id.close_date", help="Date of the merge of the first pr")
 
-    test_tags = fields.Char(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True)
-    canonical_tags = fields.Char('Canonical tag', compute='_compute_canonical_tags', store=True)
+    test_tags = fields.Text(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True)
+    canonical_tags = fields.Text('Canonical tag', compute='_compute_canonical_tags', store=True)
     tags_match_count = fields.Integer('Nb errors matching the test_tags', compute='_compute_tags_match_count')
     tags_min_version_excluded_id = fields.Many2one('runbot.version', 'Tag min version (excluded)')
     tags_min_version_id = fields.Many2one('runbot.version', 'Tags Min version', compute="_compute_tags_min_version_id", inverse="_inverse_tags_min_version_id", help="Minimal version where the test tags will be applied.", tracking=True)
@@ -288,7 +287,7 @@ def _inverse_tags_min_version_id(self):
     def _compute_canonical_tags(self):
         for record in self:
             canonical_tags = sorted(set(record.error_content_ids.filtered('canonical_tag').mapped('canonical_tag')))
-            record.canonical_tags = ','.join(canonical_tags)
+            record.canonical_tags = '\n'.join(canonical_tags)
 
     @api.depends('tags_min_version_id')
     def _compute_tags_min_version_id(self):
@@ -430,10 +429,10 @@ def _compute_unique_qualifiers(self):
     @api.depends('common_qualifiers')
     def _compute_similar_ids(self):
         for record in self:
-            if record.common_qualifiers:
+            if record.common_qualifiers and (record.id or record.id.origin):
                 query = SQL(
                     r"""SELECT id FROM runbot_build_error WHERE id != %s AND common_qualifiers @> %s""",
-                    record.id,
+                    record.id or record.id.origin,
                     json.dumps(record.common_qualifiers.dict),
                 )
                 self.env.cr.execute(query)
@@ -444,10 +443,10 @@ def _compute_similar_ids(self):
     @api.depends('common_qualifiers')
     def _compute_similar_content_ids(self):
         for record in self:
-            if record.common_qualifiers:
+            if record.common_qualifiers and (record.id or record.id.origin):
                 query = SQL(
                     r"""SELECT id FROM runbot_build_error_content WHERE error_id != %s AND qualifiers @> %s""",
-                    record.id,
+                    record.id or record.id.origin,
                     json.dumps(record.common_qualifiers.dict),
                 )
                 self.env.cr.execute(query)
@@ -458,10 +457,10 @@ def _compute_similar_content_ids(self):
     @api.depends('common_qualifiers')
     def _compute_analogous_ids(self):
         for record in self:
-            if record.common_qualifiers:
+            if record.common_qualifiers and (record.id or record.id.origin):
                 query = SQL(
                     r"""SELECT id FROM runbot_build_error WHERE id != %s AND unique_qualifiers @> %s""",
-                    record.id,
+                    record.id or record.id.origin,
                     json.dumps(record.unique_qualifiers.dict),
                 )
                 self.env.cr.execute(query)
@@ -472,10 +471,10 @@ def _compute_analogous_ids(self):
     @api.depends('common_qualifiers')
     def _compute_analogous_content_ids(self):
         for record in self:
-            if record.common_qualifiers:
+            if record.common_qualifiers and (record.id or record.id.origin):
                 query = SQL(
                     r"""SELECT id FROM runbot_build_error_content WHERE error_id != %s AND qualifiers @> %s""",
-                    record.id,
+                    record.id or record.id.origin,
                     json.dumps(record.unique_qualifiers.dict),
                 )
                 self.env.cr.execute(query)
@@ -488,16 +487,20 @@ def _compute_tags_match_count(self):
         for record in self:
             record.tags_match_count = 0
             if record.test_tags:
-                tags_parser = TestTagsParser(record.test_tags)
-                search_domain = tags_parser.test_tags_to_search_domain(exclude_error_id=record.id)
-                if search_domain:
-                    record.tags_match_count = self.env['runbot.build.error'].with_context(active_test=True).search_count(search_domain)
+                try:
+                    tags_parser = TestTagsParser(record.test_tags.replace('\n', ','))
+                    search_domain = tags_parser.test_tags_to_search_domain(exclude_error_id=record.id or record.id.origin)
+                    if search_domain:
+                        record.tags_match_count = self.env['runbot.build.error'].with_context(active_test=True).search_count(search_domain)
+                except Exception as e:  # noqa: BLE001
+                    record.tags_match_count = -1
+                    _logger.warning("Error while computing tags_match_count for error %s with test_tags %s: %s", record.id, record.test_tags, e)
 
     def action_view_impacted_by_tag(self):
         self.ensure_one()
         if not self.test_tags:
             return
-        tags_parser = TestTagsParser(self.test_tags)
+        tags_parser = TestTagsParser(self.test_tags.replace('\n', ','))
         return {
             'type': 'ir.actions.act_window',
             'views': [(False, 'list'), (False, 'form')],
@@ -510,8 +513,15 @@ def action_view_impacted_by_tag(self):
     @api.constrains('test_tags')
     def _check_test_tags(self):
         for build_error in self:
-            if build_error.test_tags and '-' in build_error.test_tags:
-                raise ValidationError('Build error test_tags should not be negated')
+            if build_error.test_tags:
+                try:
+                    test_tags = build_error.test_tags.replace('\n', ',')
+                    tags_parser = TestTagsParser(test_tags)
+                    tags_parser = TestTagsParser(test_tags, keep_escape=False)
+                except Exception as e:  # noqa: BLE001
+                    raise ValidationError(f'Invalid test_tags format: {e}')
+                if tags_parser.exclude or any(params[0] == '-' for p, params in tags_parser.parameters):
+                    raise ValidationError('Build error test_tags should not be negated')
 
     @api.onchange('test_tags')
     def _onchange_test_tags(self):
@@ -586,12 +596,12 @@ def _merge(self, others):
                     error.sudo().test_tags = previous_error.test_tags
                     previous_error.sudo().test_tags = False
                 elif self.env.su:
-                    test_tags = error.test_tags.split(',')
-                    previous_error
-                    for tag in previous_error.test_tags.split(','):
+                    test_tags = TestTagsParser(error.test_tags.replace('\n', ',')).filter_specs
+                    previous_error_tags = TestTagsParser(previous_error.test_tags.replace('\n', ',')).filter_specs
+                    for tag in previous_error_tags:
                         if tag not in test_tags:
                             test_tags.append(tag)
-                    error.test_tags = ','.join(test_tags)
+                    error.test_tags = '\n'.join(test_tags)
                     previous_error.test_tags = False
             for field in fields_to_merge + fields_to_copy:
                 if previous_error[field]:
@@ -622,7 +632,16 @@ def filter_tags(e):
             return True
 
         test_tag_list = self.search([('test_tags', '!=', False)]).filtered(filter_tags).mapped('test_tags')
-        return [test_tag for error_tags in test_tag_list for test_tag in (error_tags).split(',')]
+        parsed_test_tags = []
+        for error_tags in test_tag_list:
+            try:
+                # we cannot rely only on '\n' since old test-tags or user defined ones could be comma separated
+                error_tags = error_tags.replace('\n', ',')
+                tags_parser = TestTagsParser(error_tags)
+                parsed_test_tags.extend(tags_parser.filter_specs)
+            except Exception as e:  # noqa: BLE001
+                _logger.warning('Error while parsing test_tags for error with id %s: %s', self.id, e)
+        return parsed_test_tags
 
     @api.model
     def _disabling_tags(self, build_id=False):
@@ -853,7 +872,7 @@ class BuildErrorContent(models.Model):
     breaking_pr_id = fields.Many2one(related='error_id.breaking_pr_id')
     fixing_pr_alive = fields.Boolean(related='error_id.fixing_pr_alive')
     fixing_pr_url = fields.Char(related='error_id.fixing_pr_url')
-    test_tags = fields.Char(related='error_id.test_tags')
+    test_tags = fields.Text(related='error_id.test_tags')
     tags_min_version_id = fields.Many2one(related='error_id.tags_min_version_id')
     tags_max_version_id = fields.Many2one(related='error_id.tags_max_version_id')
 
@@ -991,10 +1010,10 @@ def _compute_error_display_id(self):
     def _compute_similar_ids(self):
         """error contents having the exactly the same qualifiers"""
         for record in self:
-            if record.qualifiers:
+            if record.qualifiers and (record.id or record.id.origin):
                 query = SQL(
                     r"""SELECT id FROM runbot_build_error_content WHERE id != %s AND qualifiers @> %s AND qualifiers <@ %s""",
-                    record.id,
+                    record.id or record.id.origin,
                     json.dumps(record.qualifiers.dict),
                     json.dumps(record.qualifiers.dict),
                 )