From ad926a0ae34873a244fea095149ce392e839b635 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Tue, 3 Sep 2024 15:25:17 +0200 Subject: [PATCH] [FIX] runbot: fix markdown adding escape A common error on runbot is to generate link containing a __init__.py file [/some/path/to/__init__.py](/some/path/to/__init__.py) This would be rendered as /some/path/to/init.py Breaking the link, and the display of the name By default markdown will not render links avoiding this issue, but it will remain for the content of the a, needing to manage some kind of escaping. The way to escape markdown is to add a \ before any special character This must be done upront before formating, adding the method markdown_escape Our implementation of markdown is not meant to meet the exact specification of markdown but better suit our needs. One of the requirements is to be able to use it to format message easily but adding dynamic countent comming from the outside. One of the error than can occur is also 'Some code `%s`' % code can also cause problem if code contains ` This issue could be solved using indented code block, but this would need to complexify the generated string, have a dedicated method to escape the code blocs, ... Since we have the controll on the input, we can easily sanitize all ynamic content to avoid such issues. For code block we introduce a way to escape backtick (\`). It is non standard but will be easier to use. Combine with that, the build._log method now allows to add args with the values to format the string (similar to logging) but will escape params by default. (cr.execute spirit) name = '__init__.py' url = 'path/to/__init__.py' code = '# comment `for` something' build._log('f', 'Some message [%s](%s) \n `%s`', name, url, code) name, url and code will be escaped --- runbot/common.py | 27 ++++++-- runbot/controllers/frontend.py | 2 +- runbot/models/batch.py | 3 +- runbot/models/build.py | 27 ++++++-- runbot/models/build_config.py | 21 +++--- runbot/models/build_config_codeowner.py | 15 +++-- runbot/models/ir_logging.py | 4 ++ runbot/templates/frontend.xml | 2 +- runbot/tests/test_build_config_step.py | 40 +++++++---- runbot/tests/test_event.py | 89 +++++++++++++++++++++++-- runbot_cla/build_config.py | 2 +- runbot_populate/models/runbot.py | 2 +- 12 files changed, 183 insertions(+), 51 deletions(-) diff --git a/runbot/common.py b/runbot/common.py index e774a193..8123e569 100644 --- a/runbot/common.py +++ b/runbot/common.py @@ -191,14 +191,17 @@ def pseudo_markdown(text): codes = [] def code_remove(match): codes.append(match.group(1)) - return f'{len(codes)-1}' + return f'{len(codes) - 1}' + + escape = r'(?\\g<1>', r'~~(.+?)~~': '\\g<1>', # it's not official markdown but who cares r'__(.+?)__': '\\g<1>', # same here, maybe we should change the method name - r'\r?\n': '
', + r'\r?\n': '
\n', } for p, b in patterns.items(): @@ -209,15 +212,31 @@ def pseudo_markdown(text): text = re_icon.sub('', text) # links - re_links = re.compile(r'\[(.+?)\]\((.+?)\)') + re_links = re.compile(rf'{escape}\[(.+?){escape}\]{escape}\(((http|/).+?{escape})\)') text = re_links.sub('\\g<1>', text) def code_replace(match): return f'{codes[int(match.group(1))]}' text = Markup(re.sub(r'(\d+)', code_replace, text, flags=re.DOTALL)) + text = markdown_unescape(text) return text +patterns = ['\\', '[', ']', '(', ')', '_', '*', '#', '`'] + +def markdown_escape(text): + text = str(text) + for pat in patterns: + text = text.replace(pat, rf'\{pat}') + return text + + +def markdown_unescape(text): + for pat in patterns: + text = text.replace(rf'\{pat}', pat) + return text + + def make_github_session(token): session = requests.Session() diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index c7980c1d..a186bc5e 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -573,7 +573,7 @@ class Runbot(Controller): builds = request.env['runbot.build'].with_context(active_test=False) if center_build_id: builds = builds.search( - expression.AND([builds_domain, [('id', '>=', center_build_id)]]), + expression.AND([builds_domain, [('id', '>=', center_build_id)]]), order='id', limit=limit/2) builds_domain = expression.AND([builds_domain, [('id', '<=', center_build_id)]]) limit -= len(builds) diff --git a/runbot/models/batch.py b/runbot/models/batch.py index bd66ef32..0d3c20c6 100644 --- a/runbot/models/batch.py +++ b/runbot/models/batch.py @@ -4,7 +4,7 @@ import datetime import subprocess from odoo import models, fields, api -from ..common import dt2time, s2human_long, pseudo_markdown +from ..common import dt2time, s2human_long, pseudo_markdown, markdown_escape _logger = logging.getLogger(__name__) @@ -462,6 +462,7 @@ class Batch(models.Model): self._log(message, *args, level='WARNING') def _log(self, message, *args, level='INFO'): + args = tuple([markdown_escape(arg) for arg in args]) message = message % args if args else message self.env['runbot.batch.log'].create({ 'batch_id': self.id, diff --git a/runbot/models/build.py b/runbot/models/build.py index 86c9d7e5..9bba2633 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -15,7 +15,7 @@ from pathlib import Path from psycopg2 import sql from psycopg2.extensions import TransactionRollbackError -from ..common import dt2time, now, grep, local_pgadmin_cursor, s2human, dest_reg, os, list_local_dbs, pseudo_markdown, RunbotException, findall, sanitize +from ..common import dt2time, now, grep, local_pgadmin_cursor, s2human, dest_reg, os, list_local_dbs, pseudo_markdown, RunbotException, findall, sanitize, markdown_escape from ..container import docker_stop, docker_state, Command, docker_run from ..fields import JsonDictField @@ -801,7 +801,7 @@ class BuildResult(models.Model): return False new_step = step_ids[next_index] # job to do, state is job_state (testing or running) if new_step.domain_filter and not self.filtered_domain(safe_eval(new_step.domain_filter)): - self._log('run', '**Skipping** step ~~%s~~ from config **%s**' % (new_step.name, self.params_id.config_id.name), log_type='markdown', level='SEPARATOR') + self._log('run', '**Skipping** step ~~%s~~ from config **%s**', (new_step.name, self.params_id.config_id.name), log_type='markdown', level='SEPARATOR') next_index += 1 continue break @@ -851,7 +851,7 @@ class BuildResult(models.Model): ro_volumes[f'/data/build/{dest}'] = source if 'image_tag' not in kwargs: kwargs.update({'image_tag': self.params_id.dockerfile_id.image_tag}) - self._log('Preparing', 'Using Dockerfile Tag [%s](/runbot/dockerfile/tag/%s)' % (kwargs['image_tag'], kwargs['image_tag']), log_type='markdown') + self._log('Preparing', 'Using Dockerfile Tag [%s](/runbot/dockerfile/tag/%s)', kwargs['image_tag'], kwargs['image_tag'], log_type='markdown') containers_memory_limit = self.env['ir.config_parameter'].sudo().get_param('runbot.runbot_containers_memory', 0) if containers_memory_limit and 'memory' not in kwargs: kwargs['memory'] = int(float(containers_memory_limit) * 1024 ** 3) @@ -972,13 +972,26 @@ class BuildResult(models.Model): local_cr.execute(sql.SQL("""CREATE DATABASE {} TEMPLATE %s LC_COLLATE 'C' ENCODING 'unicode'""").format(sql.Identifier(dbname)), (db_template,)) self.env['runbot.database'].create({'name': dbname, 'build_id': self.id}) - def _log(self, func, message, level='INFO', log_type='runbot', path='runbot'): + def _log(self, func, message, *args, level='INFO', log_type='runbot', path='runbot'): + def truncate(message, maxlenght=300000): + if len(message) > maxlenght: + return message[:maxlenght] + '[Truncate, message too long]' + return message - if len(message) > 300000: - message = message[:300000] + '[Truncate, message too long]' + if log_type == 'markdown': + args = tuple([markdown_escape(truncate(str(arg), maxlenght=200000)) for arg in args]) + + if args: + try: + message = message % args + except TypeError: + _logger.exception(f'Error while formating `{message}` with `{args}`') + message = ' ' .join([message] + args) + + message = truncate(message) self.ensure_one() - _logger.info("Build %s %s %s", self.id, func, message) + #_logger.info("Build %s %s %s", self.id, func, message) return self.env['ir.logging'].create({ 'build_id': self.id, 'level': level, diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 4beffd03..469d331f 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -8,7 +8,7 @@ import re import shlex import time from unidiff import PatchSet -from ..common import now, grep, time2str, rfind, s2human, os, RunbotException, ReProxy +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 from odoo.exceptions import UserError, ValidationError @@ -332,6 +332,7 @@ class ConfigStep(models.Model): 'rfind': rfind, 'json_loads': json.loads, 'PatchSet': PatchSet, + 'markdown_escape': markdown_escape, } def _run_python(self, build, force=False): @@ -523,7 +524,7 @@ class ConfigStep(models.Model): valid_targets |= (builds_references_by_version_id.get(next_version.id) or build.browse()) for target in valid_targets: - build._log('', 'Checking upgrade to [%s](%s)' % (target.params_id.version_id.name, target.build_url), log_type='markdown') + build._log('', 'Checking upgrade to [%s](%s)', target.params_id.version_id.name, target.build_url, log_type='markdown') for upgrade_db in upgrade_complement_step.upgrade_dbs: if not upgrade_db.min_target_version_id or upgrade_db.min_target_version_id.number <= target.params_id.version_id.number: # note: here we don't consider the upgrade_db config here @@ -660,7 +661,7 @@ class ConfigStep(models.Model): # for commit_link in target.params_id.commit_link_ids: # if commit_link.commit_id.repo_id not in repo_ids: # additionnal_commit_links |= commit_link - # build._log('', 'Adding sources from build [%s](%s)' % (target.id, target.build_url), log_type='markdown') + # build._log('', 'Adding sources from build [%s](%s)', target.id, target.build_url, log_type='markdown') child = build._add_child({ 'upgrade_to_build_id': target.id, @@ -713,7 +714,7 @@ class ConfigStep(models.Model): for commit in commit_ids: if commit.repo_id not in target_repo_ids: target_commit_ids |= commit - build._log('', 'Adding sources from build [%s](%s)' % (target.id, target.build_url), log_type='markdown') + build._log('', 'Adding sources from build [%s](%s)', target.id, target.build_url, log_type='markdown') build = build.with_context(defined_commit_ids=target_commit_ids) exports = build._checkout() @@ -772,7 +773,7 @@ class ConfigStep(models.Model): download_db_name = '%s-%s' % (dump_build.dest, download_db_suffix) zip_name = '%s.zip' % download_db_name dump_url = '%s%s' % (dump_build._http_log_url(), zip_name) - build._log('test-migration', 'Restoring dump [%s](%s) from build [%s](%s)' % (zip_name, dump_url, dump_build.id, dump_build.build_url), log_type='markdown') + build._log('test-migration', 'Restoring dump [%s](%s) from build [%s](%s)', zip_name, dump_url, dump_build.id, dump_build.build_url, log_type='markdown') restore_suffix = self.restore_rename_db_suffix or dump_db.db_suffix or suffix assert restore_suffix restore_db_name = '%s-%s' % (build.dest, restore_suffix) @@ -892,14 +893,14 @@ class ConfigStep(models.Model): if self.coverage: xml_url = '%scoverage.xml' % build._http_log_url() html_url = 'http://%s/runbot/static/build/%s/coverage/index.html' % (build.host, build.dest) - message = 'Coverage report: [xml @icon-download](%s), [html @icon-eye](%s)' % (xml_url, html_url) - build._log('end_job', message, log_type='markdown') + message = 'Coverage report: [xml @icon-download](%s), [html @icon-eye](%s)' + build._log('end_job', message, xml_url, html_url, log_type='markdown') if self.flamegraph: dat_url = '%sflame_%s.%s' % (build._http_log_url(), self.name, 'log.gz') svg_url = '%sflame_%s.%s' % (build._http_log_url(), self.name, 'svg') - message = 'Flamegraph report: [data @icon-download](%s), [svg @icon-eye](%s)' % (dat_url, svg_url) - build._log('end_job', message, log_type='markdown') + message = 'Flamegraph report: [data @icon-download](%s), [svg @icon-eye](%s)' + build._log('end_job', message, dat_url, svg_url, log_type='markdown') def _modules_to_install(self, build): return set(build._get_modules_to_test(modules_patterns=self.install_modules)) @@ -1097,7 +1098,7 @@ class ConfigStep(models.Model): build._log('make_stats', 'Getting stats from log file') log_path = build._path('logs', '%s.txt' % self.name) if not os.path.exists(log_path): - build._log('make_stats', 'Log **%s.txt** file not found' % self.name, level='INFO', log_type='markdown') + build._log('make_stats', 'Log **%s.txt** file not found', self.name, level='INFO', log_type='markdown') return try: regex_ids = self.build_stat_regex_ids diff --git a/runbot/models/build_config_codeowner.py b/runbot/models/build_config_codeowner.py index 2888fa14..0392ccc2 100644 --- a/runbot/models/build_config_codeowner.py +++ b/runbot/models/build_config_codeowner.py @@ -1,5 +1,6 @@ import re from odoo import models, fields +from ..common import markdown_escape class ConfigStep(models.Model): @@ -18,7 +19,7 @@ class ConfigStep(models.Model): build._log('', 'More than one open pr in this bundle for %s: %s' % (commit.repo_id.name, [pr.name for pr in repo_pr]), level='ERROR') build.local_result = 'ko' return {} - build._log('', 'PR [%s](%s) found for repo **%s**' % (repo_pr.dname, repo_pr.branch_url, commit.repo_id.name), log_type='markdown') + build._log('', 'PR [%s](%s) found for repo **%s**', repo_pr.dname, repo_pr.branch_url, commit.repo_id.name, log_type='markdown') pr_by_commit[commit_link] = repo_pr else: build._log('', 'No pr for repo %s, skipping' % commit.repo_id.name) @@ -124,10 +125,10 @@ class ConfigStep(models.Model): for file, file_reviewers in reviewer_per_file.items(): href = 'https://%s/blob/%s/%s' % (commit_link.branch_id.remote_id.base_url, commit_link.commit_id.name, file.split('/', 1)[-1]) if file_reviewers: - build._log('', 'Adding %s to reviewers for file [%s](%s)' % (', '.join(sorted(file_reviewers)), file, href), log_type='markdown') + build._log('', 'Adding %s to reviewers for file [%s](%s)', ', '.join(sorted(file_reviewers)), file, href, log_type='markdown') reviewers |= file_reviewers else: - build._log('', 'No reviewer for file [%s](%s)' % (file, href), log_type='markdown') + build._log('', 'No reviewer for file [%s](%s)', file, href, log_type='markdown') if reviewers: pr = pr_by_commit[commit_link] @@ -137,19 +138,19 @@ class ConfigStep(models.Model): author_skipped_teams = set(author_skippable_teams.mapped('github_team')) if author_skipped_teams: new_reviewers = new_reviewers - author_skipped_teams - build._log('', 'Skipping teams %s since author is part of the team members' % (sorted(author_skipped_teams),), log_type='markdown') + build._log('', 'Skipping teams %s since author is part of the team members', sorted(author_skipped_teams), log_type='markdown') fw_skippable_teams = skippable_teams.filtered(lambda team: team.skip_fw_pr and team.github_team in new_reviewers and pr.pr_author == fw_bot) fw_skipped_teams = set(fw_skippable_teams.mapped('github_team')) if fw_skipped_teams: new_reviewers = new_reviewers - fw_skipped_teams - build._log('', 'Skipping teams %s (ignore forwardport)' % (sorted(fw_skipped_teams),), log_type='markdown') + build._log('', 'Skipping teams %s (ignore forwardport)', sorted(fw_skipped_teams), log_type='markdown') new_reviewers = sorted(new_reviewers) - build._log('', 'Requesting review for pull request [%s](%s): %s' % (pr.dname, pr.branch_url, ', '.join(new_reviewers)), log_type='markdown') + build._log('', 'Requesting review for pull request [%s](%s): %s', pr.dname, pr.branch_url, ', '.join(new_reviewers), log_type='markdown') response = pr.remote_id._github('/repos/:owner/:repo/pulls/%s/requested_reviewers' % pr.name, {"team_reviewers": list(new_reviewers)}, ignore_errors=False) pr._update_branch_infos(response) pr['reviewers'] = ','.join(sorted(reviewers)) else: - build._log('', 'All reviewers are already on pull request [%s](%s)' % (pr.dname, pr.branch_url,), log_type='markdown') + build._log('', 'All reviewers are already on pull request [%s](%s)', pr.dname, pr.branch_url, log_type='markdown') diff --git a/runbot/models/ir_logging.py b/runbot/models/ir_logging.py index eaf45743..5063dcc9 100644 --- a/runbot/models/ir_logging.py +++ b/runbot/models/ir_logging.py @@ -7,6 +7,7 @@ from collections import defaultdict from ..common import pseudo_markdown from odoo import models, fields, tools, api from odoo.exceptions import UserError +from odoo.tools import html_escape _logger = logging.getLogger(__name__) @@ -47,6 +48,9 @@ class IrLogging(models.Model): """ Apply pseudo markdown parser for message. """ self.ensure_one() + if self.type != 'markdown': + _logger.warning('Calling _markdown on a non markdown log') + return html_escape(self.message) return pseudo_markdown(self.message) def _compute_known_error(self): diff --git a/runbot/templates/frontend.xml b/runbot/templates/frontend.xml index 806c70bc..553bd44a 100644 --- a/runbot/templates/frontend.xml +++ b/runbot/templates/frontend.xml @@ -103,7 +103,7 @@
-
diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index 812dec6a..d66568af 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -6,6 +6,7 @@ from odoo.tools import mute_logger from odoo.exceptions import UserError from odoo.addons.runbot.common import RunbotException from .common import RunbotCase +from ..common import markdown_unescape class TestBuildConfigStepCommon(RunbotCase): def setUp(self): @@ -119,24 +120,24 @@ class TestCodeowner(TestBuildConfigStepCommon): self.config_step._run_codeowner(self.parent_build) messages = self.parent_build.log_ids.mapped('message') self.assertEqual(messages[1], 'Checking 2 codeowner regexed on 3 files') - self.assertEqual(messages[2], 'Adding team_js to reviewers for file [server/file.js](https://False/blob/dfdfcfcf/file.js)') - self.assertEqual(messages[3], 'Adding team_py to reviewers for file [server/file.py](https://False/blob/dfdfcfcf/file.py)') - self.assertEqual(messages[4], 'Adding codeowner-team to reviewers for file [server/file.xml](https://False/blob/dfdfcfcf/file.xml)') - self.assertEqual(messages[5], 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_js, team_py') + self.assertEqual(markdown_unescape(messages[2]), 'Adding team_js to reviewers for file [server/file.js](https://False/blob/dfdfcfcf/file.js)') + self.assertEqual(markdown_unescape(messages[3]), 'Adding team_py to reviewers for file [server/file.py](https://False/blob/dfdfcfcf/file.py)') + self.assertEqual(markdown_unescape(messages[4]), 'Adding codeowner-team to reviewers for file [server/file.xml](https://False/blob/dfdfcfcf/file.xml)') + self.assertEqual(markdown_unescape(messages[5]), 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_js, team_py') self.assertEqual(self.dev_pr.reviewers, 'codeowner-team,team_js,team_py') def test_codeowner_regex_some_already_on(self): self.diff = 'file.js\nfile.py\nfile.xml' self.dev_pr.reviewers = 'codeowner-team,team_js' self.config_step._run_codeowner(self.parent_build) - messages = self.parent_build.log_ids.mapped('message') - self.assertEqual(messages[5], 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): team_py') + messages = self.parent_build.log_ids.mapped('message') + self.assertEqual(markdown_unescape(messages[5]), 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): team_py') def test_codeowner_regex_all_already_on(self): self.diff = 'file.js\nfile.py\nfile.xml' self.dev_pr.reviewers = 'codeowner-team,team_js,team_py' self.config_step._run_codeowner(self.parent_build) - messages = self.parent_build.log_ids.mapped('message') + messages = self.parent_build.log_ids.mapped('message') self.assertEqual(messages[5], 'All reviewers are already on pull request [base/server:1234](https://example.com/base/server/pull/1234)') def test_codeowner_author_in_team(self): @@ -147,8 +148,8 @@ class TestCodeowner(TestBuildConfigStepCommon): self.dev_pr.pr_author = 'some_member' self.config_step._run_codeowner(self.parent_build) messages = self.parent_build.log_ids.mapped('message') - self.assertEqual(messages[5], "Skipping teams ['team_py'] since author is part of the team members") - self.assertEqual(messages[6], 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_js') + self.assertEqual(markdown_unescape(messages[5]), "Skipping teams ['team_py'] since author is part of the team members") + self.assertEqual(markdown_unescape(messages[6]), 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_js') self.assertEqual(self.dev_pr.reviewers, 'codeowner-team,team_js,team_py') def test_codeowner_ownership_base(self): @@ -160,7 +161,7 @@ class TestCodeowner(TestBuildConfigStepCommon): self.config_step._run_codeowner(self.parent_build) messages = self.parent_build.log_ids.mapped('message') self.assertEqual( - messages[2], + markdown_unescape(messages[2]), 'Adding team_01, team_py to reviewers for file [server/core/addons/module1/some/file.py](https://False/blob/dfdfcfcf/core/addons/module1/some/file.py)' ) @@ -173,7 +174,7 @@ class TestCodeowner(TestBuildConfigStepCommon): self.config_step._run_codeowner(self.parent_build) messages = self.parent_build.log_ids.mapped('message') self.assertEqual( - messages[2], + markdown_unescape(messages[2]), 'Adding team_py to reviewers for file [server/core/addons/module1/some/file.py](https://False/blob/dfdfcfcf/core/addons/module1/some/file.py)' ) @@ -189,7 +190,7 @@ class TestCodeowner(TestBuildConfigStepCommon): 'core/addons/module4/some/file.txt', ]) self.config_step._run_codeowner(self.parent_build) - messages = self.parent_build.log_ids.mapped('message') + messages = [markdown_unescape(message) for message in self.parent_build.log_ids.mapped('message')] self.assertEqual(messages, [ 'PR [base/server:1234](https://example.com/base/server/pull/1234) found for repo **server**', 'Checking 2 codeowner regexed on 4 files', @@ -200,6 +201,21 @@ class TestCodeowner(TestBuildConfigStepCommon): 'Requesting review for pull request [base/server:1234](https://example.com/base/server/pull/1234): codeowner-team, team_01, team_02, team_js, team_py' ]) + def test_codeowner___init__log(self): + module1 = self.env['runbot.module'].create({'name': "module1"}) + self.env['runbot.module.ownership'].create({'team_id': self.team1.id, 'module_id': module1.id}) + self.diff = '\n'.join([ + 'core/addons/module1/some/__init__.py', + ]) + self.config_step._run_codeowner(self.parent_build) + logs = self.parent_build.log_ids + print + self.assertEqual( + logs[2]._markdown(), + 'Adding team_01, team_py to reviewers for file server/core/addons/module1/some/__init__.py', + '__init__.py should not be replaced by init.py' + ) + class TestBuildConfigStepRestore(TestBuildConfigStepCommon): @classmethod diff --git a/runbot/tests/test_event.py b/runbot/tests/test_event.py index a969741b..685ea3ef 100644 --- a/runbot/tests/test_event.py +++ b/runbot/tests/test_event.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +from ..common import markdown_escape, markdown_unescape from .common import RunbotCase @@ -7,7 +8,7 @@ class TestIrLogging(RunbotCase): def test_markdown(self): log = self.env['ir.logging'].create({ 'name': 'odoo.runbot', - 'type': 'runbot', + 'type': 'markdown', 'path': 'runbot', 'level': 'INFO', 'line': 0, @@ -26,10 +27,16 @@ class TestIrLogging(RunbotCase): # 'a bit of code import foo\nfoo.bar' #) + log.message = '`import foo`' + self.assertEqual( + str(log._markdown()), + 'import foo', + ) + log.message = 'a bit of code :\n`import foo`' self.assertEqual( - log._markdown(), - 'a bit of code :
import foo' + str(log._markdown()), + 'a bit of code :
\nimport foo', ) @@ -43,20 +50,20 @@ class TestIrLogging(RunbotCase): log.message = 'a bit of code :\n`print(__name__)`' self.assertEqual( log._markdown(), - 'a bit of code :
print(__name__)' + 'a bit of code :
\nprint(__name__)' ) log.message = 'a bit of __code__ :\n`print(__name__)` **but also** `print(__name__)`' self.assertEqual( log._markdown(), - 'a bit of code :
print(__name__) but also print(__name__)' + 'a bit of code :
\nprint(__name__) but also print(__name__)' ) # test links log.message = 'This [link](https://wwww.somewhere.com) goes to somewhere and [this one](http://www.nowhere.com) to nowhere.' self.assertEqual( - log._markdown(), + str(log._markdown()), 'This link goes to somewhere and this one to nowhere.' ) @@ -80,3 +87,73 @@ class TestIrLogging(RunbotCase): log._markdown(), 'foo <script>console.log("hello world")</script>' ) + + log.message = f'[file]({markdown_escape("https://repo/file/__init__.py")})' + self.assertEqual( + str(log._markdown()), + 'file', + ) + + # BEHAVIOUR TO DEFINE + + log.message = f'[__underline text__]({markdown_escape("https://repo/file/__init__.py")})' + self.assertEqual( + str(log._markdown()), + 'underline text', + ) + + # BEHAVIOUR TO DEFINE + log.message = f'[{markdown_escape("__init__.py")}]({markdown_escape("https://repo/file/__init__.py")})' + self.assertEqual( + str(log._markdown()), + '__init__.py', + ) + + log.message = f'''This is a list of failures in some files: +[{markdown_escape("__init__.py")}]({markdown_escape("https://repo/file/__init__.py")}) +`{markdown_escape("Some code with talking about __enter__")}` +[{markdown_escape("__init__.py")}]({markdown_escape("https://repo/file/__init__.py")}) +`{markdown_escape("Some code with `code block` inside")}`''' + + self.assertEqual( + str(log._markdown()), + '''This is a list of failures in some files:
+__init__.py
+Some code with talking about __enter__
+__init__.py
+Some code with `code block` inside''') + + for code in [ + 'leading\\', + 'leading\\\\', + '`', + '\\`', + '\\``', + '``', + '`\n`', + ]: + escaped_code = markdown_escape(code) + + log.message = f'This is a bloc code `{escaped_code}`' + self.assertEqual( + str(log._markdown()), + f'This is a bloc code {code}', + ) + + def test_build_log_markdown(self): + build = self.env['runbot.build'].create({'params_id': self.base_params.id}) + + def last_log(): + return str(build.log_ids[-1]._markdown()) + + name = '__init__.py' + url = '/path/to/__init__.py' + code = '# comment `for` something' + build._log('f', 'Some message [%s](%s) \n `%s`', name, url, code, log_type='markdown') + self.assertEqual(last_log(), f'Some message {name}
\n {code}') + + name = '[__init__.py]' + url = '/path/to/__init__.py=(test))' + code = '# comment `for` something\\' + build._log('f', 'Some message [%s](%s) \n `%s`', name, url, code, log_type='markdown') + self.assertEqual(last_log(), f'Some message {name}
\n {code}') diff --git a/runbot_cla/build_config.py b/runbot_cla/build_config.py index ab506ef5..c78fbb57 100644 --- a/runbot_cla/build_config.py +++ b/runbot_cla/build_config.py @@ -26,7 +26,7 @@ class Step(models.Model): if email in checked: continue checked.add(email) - build._log('check_cla', "[Odoo CLA signature](https://www.odoo.com/sign-cla) check for %s (%s) " % (commit.author, email), log_type='markdown') + build._log('check_cla', "[Odoo CLA signature](https://www.odoo.com/sign-cla) check for %s (%s) ", commit.author, email, log_type='markdown') mo = re.search('[^ <@]+@[^ @>]+', email or '') if mo: email = mo.group(0).lower() diff --git a/runbot_populate/models/runbot.py b/runbot_populate/models/runbot.py index 9c437366..6fa457ce 100644 --- a/runbot_populate/models/runbot.py +++ b/runbot_populate/models/runbot.py @@ -133,7 +133,7 @@ class Runbot(models.AbstractModel): build._log('******','Starting step Y', level='SEPARATOR') build._log('******','Some log', level='ERROR') build._log('******','Some log\n with multiple lines', level='ERROR') - build._log('******','**Some** *markdown* [log](http://example.com)', log_type='markdown') + build._log('******','**Some** *markdown* [log](%s)', 'http://example.com', log_type='markdown') build._log('******','Step x finished', level='SEPARATOR') build.local_state = 'done'