From 926ab4a9d90c391e41b58c492fb610bdbf711976 Mon Sep 17 00:00:00 2001
From: Christophe Monniez <moc@odoo.com>
Date: Mon, 6 Jan 2020 10:19:58 +0100
Subject: [PATCH] [FIX] runbot: fix build_error test tags validation

When trying to remove test_tags on a build_error, the validation fails
because it tries to iterate on False. Also, the ValidationError
exception was not properly imported.

With this commit, the validation is fixed and a test is added.
---
 runbot/models/build_error.py     |  6 +++---
 runbot/tests/test_build_error.py | 34 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/runbot/models/build_error.py b/runbot/models/build_error.py
index 03c478e3..723d2052 100644
--- a/runbot/models/build_error.py
+++ b/runbot/models/build_error.py
@@ -5,6 +5,7 @@ import re
 
 from collections import defaultdict
 from odoo import models, fields, api
+from odoo.exceptions import ValidationError
 
 _logger = logging.getLogger(__name__)
 
@@ -40,11 +41,10 @@ class RunbotBuildError(models.Model):
     last_seen_date = fields.Datetime(string='Last Seen Date', related='last_seen_build_id.create_date')
     test_tags = fields.Char(string='Test tags', help="Comma separated list of test_tags to use to reproduce/remove this error")
 
-
     @api.constrains('test_tags')
     def _check_test_tags(self):
-        for step in self:
-            if '-' in step.test_tags:
+        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')
 
     @api.model
diff --git a/runbot/tests/test_build_error.py b/runbot/tests/test_build_error.py
index ff2059b1..bb68513b 100644
--- a/runbot/tests/test_build_error.py
+++ b/runbot/tests/test_build_error.py
@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from unittest.mock import patch
 from odoo.tests import common
+from odoo.exceptions import ValidationError
 from .common import RunbotCase
 
 RTE_ERROR = """FAIL: TestUiTranslate.test_admin_tour_rte_translator
@@ -128,3 +129,36 @@ class TestBuildError(RunbotCase):
         self.assertIn(build_a, error_b.children_build_ids)
         self.assertEqual(error_a.build_count, 1)
         self.assertEqual(error_b.build_count, 2)
+
+    def test_build_error_test_tags(self):
+        build_a = self.create_test_build({'local_result': 'ko'})
+        build_b = self.create_test_build({'local_result': 'ko'})
+
+        error_a = self.BuildError.create({
+            'content': 'foo',
+            'build_ids': [(6, 0, [build_a.id])],
+            'random': True,
+            'active': True
+        })
+
+        error_b = self.BuildError.create({
+            'content': 'bar',
+            'build_ids': [(6, 0, [build_b.id])],
+            'random': True,
+            'active': False
+        })
+
+        # test that a test tag with a dash raise an Vamlidation error
+        with self.assertRaises(ValidationError):
+            error_a.test_tags = '-foo'
+
+        error_a.test_tags = 'foo,bar'
+        error_b.test_tags = 'blah'
+        self.assertIn('foo', self.BuildError.test_tags_list())
+        self.assertIn('bar', self.BuildError.test_tags_list())
+        self.assertIn('-foo', self.BuildError.disabling_tags())
+        self.assertIn('-bar', self.BuildError.disabling_tags())
+
+        # test that test tags on fixed errors are not taken into account
+        self.assertNotIn('blah', self.BuildError.test_tags_list())
+        self.assertNotIn('-blah', self.BuildError.disabling_tags())