[IMP] runbot: use real log date on build errors

In the build error view, a list of build is displayed with a confusing
create date. The create date in the list is the creation date of the
build, leading to a confusion with the creation of the build log
creation.

With this commit, the real log creation is used in this view.

To achieve that, the many2many relation is extended with a
log_date which is filled when a build log entry is parsed.

runbot/__manifest__.py

@@ -2,11 +2,11 @@
 {
     'name': "runbot",
     'summary': "Runbot",
-    'description': "Runbot for Odoo 15.0",
+    'description': "Runbot for Odoo 16.0",
     'author': "Odoo SA",
     'website': "http://runbot.odoo.com",
     'category': 'Website',
-    'version': '5.4',
+    'version': '5.5',
     'application': True,
     'depends': ['base', 'base_automation', 'website'],
     'data': [

runbot/migrations/16.0.5.5/post-migration.py

@@ -0,0 +1,8 @@
+def migrate(cr, version):
+    cr.execute(
+    """
+        INSERT INTO runbot_build_error_link(build_id,build_error_id,log_date)
+             SELECT runbot_build_id,runbot_build_error_id,runbot_build.create_date as create_date 
+               FROM runbot_build_error_ids_runbot_build_rel 
+          LEFT JOIN runbot_build ON runbot_build.id = runbot_build_id;
+    """)

runbot/models/build.py

@@ -219,7 +219,8 @@ class BuildResult(models.Model):
     orphan_result = fields.Boolean('No effect on the parent result', default=False)
 
     build_url = fields.Char('Build url', compute='_compute_build_url', store=False)
-    build_error_ids = fields.Many2many('runbot.build.error', 'runbot_build_error_ids_runbot_build_rel', string='Errors')
+    build_error_link_ids = fields.One2many('runbot.build.error.link', 'build_id')
+    build_error_ids = fields.Many2many('runbot.build.error', compute='_compute_build_error_ids', string='Errors')
     keep_running = fields.Boolean('Keep running', help='Keep running', index=True)
     log_counter = fields.Integer('Log Lines counter', default=100)
 
@@ -311,6 +312,11 @@ class BuildResult(models.Model):
                 else:
                     record.global_result = record.local_result
 
+    @api.depends('build_error_link_ids')
+    def _compute_build_error_ids(self):
+        for record in self:
+            record.build_error_ids = record.build_error_link_ids.mapped('build_error_id')
+
     def _get_worst_result(self, results, max_res=False):
         results = [result for result in results if result]  # filter Falsy values
         index = max([self._get_result_score(result) for result in results]) if results else 0
@@ -1128,7 +1134,7 @@ class BuildResult(models.Model):
         """ Parse build logs to classify errors """
         BuildError = self.env['runbot.build.error']
         # only parse logs from builds in error and not already scanned
-        builds_to_scan = self.search([('id', 'in', self.ids), ('local_result', 'in', ('ko', 'killed', 'warn')), ('build_error_ids', '=', False)])
+        builds_to_scan = self.search([('id', 'in', self.ids), ('local_result', 'in', ('ko', 'killed', 'warn')), ('build_error_link_ids', '=', False)])
         ir_logs = self.env['ir.logging'].search([('level', 'in', ('ERROR', 'WARNING', 'CRITICAL')), ('type', '=', 'server'), ('build_id', 'in', builds_to_scan.ids)])
         return BuildError._parse_logs(ir_logs)
 

runbot/models/build_error.py

@@ -4,6 +4,8 @@ import hashlib
 import logging
 import re
 
+from psycopg2 import sql
+
 from collections import defaultdict
 from dateutil.relativedelta import relativedelta
 from werkzeug.urls import url_join
@@ -13,6 +15,26 @@ from odoo.exceptions import ValidationError, UserError
 _logger = logging.getLogger(__name__)
 
 
+class BuildErrorLink(models.Model):
+    _name = 'runbot.build.error.link'
+    _description = 'Build Build Error Extended Relation'
+    _order = 'log_date desc, build_id desc'
+
+    build_id = fields.Many2one('runbot.build', required=True, index=True)
+    build_error_id =fields.Many2one('runbot.build.error', required=True, index=True)
+    log_date = fields.Datetime(string='Log date')
+    host = fields.Char(related='build_id.host')
+    dest = fields.Char(related='build_id.dest')
+    version_id = fields.Many2one(related='build_id.version_id')
+    trigger_id = fields.Many2one(related='build_id.trigger_id')
+    description = fields.Char(related='build_id.description')
+    build_url = fields.Char(related='build_id.build_url')
+
+    _sql_constraints = [
+        ('error_build_rel_unique', 'UNIQUE (build_id, build_error_id)', 'A link between a build and an error must be unique'),
+    ]
+
+
 class BuildError(models.Model):
 
     _name = "runbot.build.error"
@@ -35,7 +57,9 @@ class BuildError(models.Model):
     fixing_pr_id = fields.Many2one('runbot.branch', 'Fixing PR', tracking=True, domain=[('is_pr', '=', True)])
     fixing_pr_alive = fields.Boolean('Fixing PR alive', related='fixing_pr_id.alive')
     fixing_pr_url = fields.Char('Fixing PR url', related='fixing_pr_id.branch_url')
-    build_ids = fields.Many2many('runbot.build', 'runbot_build_error_ids_runbot_build_rel', string='Affected builds')
+    build_error_link_ids = fields.One2many('runbot.build.error.link', 'build_error_id')
+    children_build_error_link_ids = fields.One2many('runbot.build.error.link', compute='_compute_children_build_error_link_ids')
+    build_ids = fields.Many2many('runbot.build', compute= '_compute_build_ids')
     bundle_ids = fields.One2many('runbot.bundle', compute='_compute_bundle_ids')
     version_ids = fields.One2many('runbot.version', compute='_compute_version_ids', string='Versions', search='_search_version')
     trigger_ids = fields.Many2many('runbot.trigger', compute='_compute_trigger_ids', string='Triggers', search='_search_trigger_ids')
@@ -47,9 +71,9 @@ class BuildError(models.Model):
     children_build_ids = fields.Many2many('runbot.build', compute='_compute_children_build_ids', string='Children builds')
     error_history_ids = fields.Many2many('runbot.build.error', compute='_compute_error_history_ids', string='Old errors', context={'active_test': False})
     first_seen_build_id = fields.Many2one('runbot.build', compute='_compute_first_seen_build_id', string='First Seen build')
-    first_seen_date = fields.Datetime(string='First Seen Date', related='first_seen_build_id.create_date', depends=['first_seen_build_id'])
+    first_seen_date = fields.Datetime(string='First Seen Date', compute='_compute_seen_date', store=True)
     last_seen_build_id = fields.Many2one('runbot.build', compute='_compute_last_seen_build_id', string='Last Seen build', store=True)
-    last_seen_date = fields.Datetime(string='Last Seen Date', related='last_seen_build_id.create_date', store=True, depends=['last_seen_build_id'])
+    last_seen_date = fields.Datetime(string='Last Seen Date', compute='_compute_seen_date', store=True)
     test_tags = fields.Char(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error", tracking=True)
 
     @api.constrains('test_tags')
@@ -105,6 +129,16 @@ class BuildError(models.Model):
                     build_error.team_id = False
         return result
 
+    @api.depends('build_error_link_ids')
+    def _compute_build_ids(self):
+        for record in self:
+            record.build_ids = record.build_error_link_ids.mapped('build_id')
+
+    @api.depends('build_error_link_ids')
+    def _compute_children_build_error_link_ids(self):
+        for record in self:
+            record.children_build_error_link_ids = record.build_error_link_ids | record.child_ids.build_error_link_ids
+
     @api.depends('build_ids', 'child_ids.build_ids')
     def _compute_build_counts(self):
         for build_error in self:
@@ -131,6 +165,7 @@ class BuildError(models.Model):
         for build_error in self:
             build_error.summary = build_error.content[:80]
 
+
     @api.depends('build_ids', 'child_ids.build_ids')
     def _compute_children_build_ids(self):
         for build_error in self:
@@ -142,6 +177,13 @@ class BuildError(models.Model):
         for build_error in self:
             build_error.last_seen_build_id = build_error.children_build_ids and build_error.children_build_ids[0] or False
 
+    @api.depends('build_error_link_ids', 'child_ids')
+    def _compute_seen_date(self):
+        for build_error in self:
+            error_dates = (build_error.build_error_link_ids | build_error.child_ids.build_error_link_ids).mapped('log_date')
+            build_error.first_seen_date = error_dates and min(error_dates)
+            build_error.last_seen_date = error_dates and max(error_dates)
+
     @api.depends('children_build_ids')
     def _compute_first_seen_build_id(self):
         for build_error in self:
@@ -179,8 +221,11 @@ class BuildError(models.Model):
         build_errors |= existing_errors
         for build_error in existing_errors:
             logs = hash_dict[build_error.fingerprint]
-            for build in logs.mapped('build_id'):
-                build.build_error_ids += build_error
+            self.env['runbot.build.error.link'].create([{
+                    'build_id': rec.build_id.id,
+                    'build_error_id': build_error.id,
+                    'log_date': rec.create_date}
+            for rec in logs if rec.build_id not in build_error.build_ids])
 
             # update filepath if it changed. This is optionnal and mainly there in case we adapt the OdooRunner log 
             if logs[0].path != build_error.file_path:
@@ -191,13 +236,19 @@ class BuildError(models.Model):
 
         # create an error for the remaining entries
         for fingerprint, logs in hash_dict.items():
-            build_errors |= self.env['runbot.build.error'].create({
+            new_build_error = self.env['runbot.build.error'].create({
                 'content': logs[0].message,
                 'module_name': logs[0].name,
                 'file_path': logs[0].path,
                 'function': logs[0].func,
-                'build_ids': [(6, False, [r.build_id.id for r in logs])],
             })
+            build_errors |= new_build_error
+            self.env['runbot.build.error.link'].create([{
+                'build_id': rec.build_id.id,
+                'build_error_id': new_build_error.id,
+                'log_date': rec.create_date}
+            for rec in logs])
+
 
         if build_errors:
             window_action = {

runbot/security/ir.model.access.csv

@@ -22,6 +22,9 @@ access_runbot_config_step_upgrade_db_manager,runbot_config_step_upgrade_db_manag
 access_runbot_build_error_user,runbot_build_error_user,runbot.model_runbot_build_error,group_user,1,0,0,0
 access_runbot_build_error_admin,runbot_build_error_admin,runbot.model_runbot_build_error,runbot.group_runbot_admin,1,1,1,1
 access_runbot_build_error_manager,runbot_build_error_manager,runbot.model_runbot_build_error,runbot.group_runbot_error_manager,1,1,1,1
+access_runbot_build_error_link_user,runbot_runbot_build_error_link_user,runbot.model_runbot_build_error_link,group_user,1,0,0,0
+access_runbot_build_error_link_admin,runbot_runbot_build_error_link_admin,runbot.model_runbot_build_error_link,runbot.group_runbot_admin,1,1,1,0
+access_runbot_build_error_link_manager,runbot_runbot_build_error_link_manager,runbot.model_runbot_build_error_link,runbot.group_runbot_error_manager,1,1,1,0
 access_runbot_build_error_tag_user,runbot_build_error_tag_user,runbot.model_runbot_build_error_tag,group_user,1,0,0,0
 access_runbot_build_error_tag_admin,runbot_build_error_tag_admin,runbot.model_runbot_build_error_tag,runbot.group_runbot_admin,1,1,1,1
 access_runbot_build_error_tag_manager,runbot_build_error_tag_manager,runbot.model_runbot_build_error_tag,runbot.group_runbot_error_manager,1,1,1,1

runbot/tests/test_build_error.py

@@ -1,5 +1,6 @@
 import hashlib
 
+from odoo import fields
 from odoo.exceptions import ValidationError
 from .common import RunbotCase
 
@@ -30,6 +31,7 @@ class TestBuildError(RunbotCase):
     def setUp(self):
         super(TestBuildError, self).setUp()
         self.BuildError = self.env['runbot.build.error']
+        self.BuildErrorLink = self.env['runbot.build.error.link']
         self.BuildErrorTeam = self.env['runbot.team']
         self.ErrorRegex = self.env['runbot.error.regex']
 
@@ -62,10 +64,12 @@ class TestBuildError(RunbotCase):
 
     def test_merge(self):
         build_a = self.create_test_build({'local_result': 'ko', 'local_state': 'done'})
-        error_a = self.BuildError.create({'content': 'foo bar', 'build_ids': [(6, 0, [build_a.id])]})
+        error_a = self.BuildError.create({'content': 'foo bar'})
+        self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_a.id})
 
         build_b = self.create_test_build({'local_result': 'ko', 'local_state': 'done'})
-        error_b = self.BuildError.create({'content': 'foo bar', 'build_ids': [(6, 0, [build_b.id])]})
+        error_b = self.BuildError.create({'content': 'foo bar'})
+        self.BuildErrorLink.create({'build_id': build_b.id, 'build_error_id': error_b.id})
 
         (error_a | error_b)._merge()
         self.assertEqual(len(self.BuildError.search([('fingerprint', '=', error_a.fingerprint)])), 1)
@@ -85,10 +89,13 @@ class TestBuildError(RunbotCase):
         top_error = self.BuildError.create({'content': 'foo foo', 'active': False})
 
         build_a = self.create_test_build({'local_result': 'ko', 'local_state': 'done'})
-        error_a = self.BuildError.create({'content': 'foo bar', 'parent_id': top_error.id , 'build_ids': [(6, 0, [build_a.id])]})
+        error_a = self.BuildError.create({'content': 'foo bar', 'parent_id': top_error.id })
+        self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_a.id})
 
         build_b = self.create_test_build({'local_result': 'ko', 'local_state': 'done'})
-        error_b = self.BuildError.create({'content': 'foo bar', 'test_tags': 'footag', 'build_ids': [(6, 0, [build_b.id])]})
+        error_b = self.BuildError.create({'content': 'foo bar', 'test_tags': 'footag'})
+        self.BuildErrorLink.create({'build_id': build_b.id, 'build_error_id': error_b.id})
+
         linked_error = self.BuildError.create({'content': 'foo foo bar', 'parent_id': error_b.id})
 
         (error_a | error_b)._merge()
@@ -124,6 +131,7 @@ class TestBuildError(RunbotCase):
         })
 
         log = {
+            'create_date': fields.Datetime.from_string('2023-08-29 00:46:21'),
             'message': RTE_ERROR,
             'build_id': ko_build.id,
             'level': 'ERROR',
@@ -144,12 +152,17 @@ class TestBuildError(RunbotCase):
 
         ko_build._parse_logs()
         ok_build._parse_logs()
-        build_error = self.BuildError.search([('build_ids', 'in', [ko_build.id])])
+        # build_error = self.BuildError.search([('build_ids', 'in', [ko_build.id])])
+        build_error = self.BuildErrorLink.search([('build_id.id', '=', ko_build.id)]).mapped('build_error_id')
         self.assertTrue(build_error)
         self.assertTrue(build_error.fingerprint.startswith('af0e88f3'))
         self.assertTrue(build_error.cleaned_content.startswith('%'), 'The cleaner should have replace "FAIL: " with a "%" sign by default')
+        error_link = self.env['runbot.build.error.link'].search([('build_id', '=', ko_build.id), ('build_error_id', '=', build_error.id)])
+        self.assertTrue(error_link, 'An error link should exists')
+        self.assertIn(ko_build, build_error.build_error_link_ids.mapped('build_id'), 'Ko build should be in build_error_link_ids')
+        self.assertEqual(error_link.log_date, fields.Datetime.from_string('2023-08-29 00:46:21'))
         self.assertIn(ko_build, build_error.build_ids, 'The parsed build should be added to the runbot.build.error')
-        self.assertFalse(self.BuildError.search([('build_ids', 'in', [ok_build.id])]), 'A successful build should not associated to a runbot.build.error')
+        self.assertFalse(self.BuildErrorLink.search([('build_id', '=', ok_build.id)]), 'A successful build should not be associated to a runbot.build.error')
         self.assertEqual(error_team, build_error.team_id)
 
         # Test that build with same error is added to the errors
@@ -175,7 +188,7 @@ class TestBuildError(RunbotCase):
         IrLog.create(log)
         ko_build_new._parse_logs()
         self.assertNotIn(ko_build_new, build_error.build_ids, 'The parsed build should not be added to a fixed runbot.build.error')
-        new_build_error = self.BuildError.search([('build_ids', 'in', [ko_build_new.id])])
+        new_build_error = self.BuildErrorLink.search([('build_id', '=', ko_build_new.id)]).mapped('build_error_id')
         self.assertIn(ko_build_new, new_build_error.build_ids, 'The parsed build with a re-apearing error should generate a new runbot.build.error')
         self.assertIn(build_error, new_build_error.error_history_ids, 'The old error should appear in history')
 
@@ -185,16 +198,18 @@ class TestBuildError(RunbotCase):
 
         error_a = self.env['runbot.build.error'].create({
             'content': 'foo',
-            'build_ids': [(6, 0, [build_a.id])],
             'active': False  # Even a fixed error coul be linked
         })
 
+        self.BuildErrorLink.create({'build_id': build_a.id, 'build_error_id': error_a.id})
+
         error_b = self.env['runbot.build.error'].create({
             'content': 'bar',
-            'build_ids': [(6, 0, [build_b.id])],
             'random': True
         })
 
+        self.BuildErrorLink.create({'build_id': build_b.id, 'build_error_id': error_b.id})
+
         #  test that the random bug is parent when linking errors
         all_errors = error_a | error_b
         all_errors.action_link_errors()

runbot/views/build_error_views.xml

@@ -46,14 +46,15 @@
                 </group>
                 <notebook>
                   <page string="Builds">
-                    <field name="children_build_ids" widget="many2many" options="{'not_delete': True, 'no_create': True}" readonly="1">
+                    <field name="children_build_error_link_ids" widget="one2many" options="{'not_delete': True, 'no_create': True}" readonly="1">
                       <tree>
-                        <field name="create_date"/>
+                        <field name="log_date"/>
                         <field name="host" groups="base.group_no_one"/>
                         <field name="dest"/>
                         <field name="version_id"/>
                         <field name="trigger_id"/>
                         <field name="description"/>
+                        <field name="build_error_id" widget="many2one" string="Linked Error"/>
                         <field name="build_url" widget="url" readonly="1" text="View build"/>
                       </tree>
                     </field>