From b8f9b91db5ae1b74d1904d3083ab82ff00e967b5 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Tue, 11 Mar 2025 12:21:05 +0100 Subject: [PATCH] [IMP] runbot: support test tags with spaces Before this commit, the runbot only supported test tags without spaces. With the introduction of parametric tags, it is possible to have test tags with the form .test_method[some text] We need to ensure that test_tags are wrapped in quotes --- runbot/container.py | 31 +++++++++++++++++++------------ runbot/models/build_config.py | 2 +- runbot/models/repo.py | 3 ++- runbot/tests/test_command.py | 12 +++++++++++- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/runbot/container.py b/runbot/container.py index 4134c9d3..c2c133db 100644 --- a/runbot/container.py +++ b/runbot/container.py @@ -64,20 +64,28 @@ class Command(): return Command(self.pres, self.cmd + l, self.posts, self.finals, self.config_tuples, self.cmd_checker) def __str__(self): - return ' '.join(self) + return self.shell_join(self.cmd) def __repr__(self): return self.build().replace('&& ', '&&\n').replace('|| ', '||\n\t').replace(';', ';\n') + + def shell_join(self, elems): + return ' '.join([self.escape(elem) for elem in elems]) + + def escape(self, elem): + if not ' ' in elem or '"' in elem: + return elem + return f'"{elem}"' def build(self): if self.cmd_checker: self.cmd_checker._cmd_check(self) cmd_chain = [] - cmd_chain += [' '.join(pre) for pre in self.pres if pre] - cmd_chain.append(' '.join(self)) - cmd_chain += [' '.join(post) for post in self.posts if post] + cmd_chain += [self.shell_join(pre) for pre in self.pres if pre] + cmd_chain += [self.shell_join(self.cmd)] + cmd_chain += [self.shell_join(post) for post in self.posts if post] cmd_chain = [' && '.join(cmd_chain)] - cmd_chain += [' '.join(final) for final in self.finals if final] + cmd_chain += [self.shell_join(final) for final in self.finals if final] return ' ; '.join(cmd_chain) def add_config_tuple(self, option, value): @@ -223,23 +231,22 @@ def _docker_run(cmd=False, log_path=False, build_dir=False, container_name=False :params env_variables: list of environment variables """ assert cmd and log_path and build_dir and container_name - run_cmd = cmd image_tag = image_tag or 'odoo:DockerDefault' container_name = sanitize_container_name(container_name) - if isinstance(run_cmd, Command): - cmd_object = run_cmd - run_cmd = cmd_object.build() + cmd_str = str(cmd) + if isinstance(cmd, Command): + run_cmd = cmd.build() else: - cmd_object = Command([], run_cmd.split(' '), []) + run_cmd = cmd + run_cmd = f'cd /data/build;touch start-{container_name};{run_cmd};cd /data/build;touch end-{container_name}' _logger.info('Docker run command: %s', run_cmd) - run_cmd = 'cd /data/build;touch start-%s;%s;cd /data/build;touch end-%s' % (container_name, run_cmd, container_name) docker_clear_state(container_name, build_dir) # ensure that no state are remaining build_dir = file_path(build_dir) file_path(os.path.dirname(log_path)) open(os.path.join(build_dir, 'exist-%s' % container_name), 'w+').close() logs = open(log_path, 'w') - logs.write("Docker command:\n%s\n=================================================\n" % cmd_object) + logs.write("Docker command:\n%s\n=================================================\n" % cmd_str) # create start script volumes = { '/var/run/postgresql': {'bind': '/var/run/postgresql', 'mode': 'rw'}, diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index 0a4aa155..51d282ac 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -777,7 +777,7 @@ class ConfigStep(models.Model): migrate_db_name = '%s-%s' % (build.dest, db_suffix) # only ok if restore does not force db_suffix migrate_cmd = build._cmd(enable_log_db=self.enable_log_db) - migrate_cmd += ['-u all'] + migrate_cmd += ['-u', 'all'] migrate_cmd += ['-d', migrate_db_name] migrate_cmd += ['--stop-after-init'] migrate_cmd += ['--max-cron-threads=0'] diff --git a/runbot/models/repo.py b/runbot/models/repo.py index 2f68d7eb..fbf488cb 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -8,6 +8,7 @@ import subprocess import time import requests import markupsafe +import shlex from pathlib import Path @@ -486,7 +487,7 @@ class Repo(models.Model): def _git(self, cmd, errors='strict'): cmd = self._get_git_command(cmd, errors) - _logger.info("git command: %s", ' '.join(cmd)) + _logger.info("git command: %s", shlex.join(cmd)) return subprocess.check_output(cmd, stderr=subprocess.STDOUT).decode(errors=errors) def _fetch(self, sha): diff --git a/runbot/tests/test_command.py b/runbot/tests/test_command.py index 6583a4ab..649b42e2 100644 --- a/runbot/tests/test_command.py +++ b/runbot/tests/test_command.py @@ -12,9 +12,16 @@ foo = bar class Test_Command(common.TransactionCase): def test_command(self): + cmd = Command([], ['python3', 'odoo-bin', '--test-tags', "-.test_js[core > utils,core > display]"], []) + self.assertEqual(cmd.build(), 'python3 odoo-bin --test-tags "-.test_js[core > utils,core > display]"') + + cmd = Command([], ['psql', '-c', '"SQL query"', '>', 'some_file'], []) + self.assertEqual(cmd.build(), 'psql -c "SQL query" > some_file') + + pres = ['pip3', 'install', 'foo'] posts = ['python3', '-m', 'coverage', 'html'] - finals = ['pgdump bar'] + finals = ['pgdump', 'bar'] cmd = Command([pres], ['python3', 'odoo-bin'], [posts], finals=[finals]) self.assertEqual(str(cmd), 'python3 odoo-bin') @@ -34,6 +41,9 @@ class Test_Command(common.TransactionCase): self.assertIn('a = b', content) self.assertIn('x = y', content) + + + with self.assertRaises(AssertionError): cmd.add_config_tuple('http-interface', '127.0.0.1')