diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9aa6f910dc..307403bdac 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -119,7 +119,6 @@ repos: src/aiida/engine/processes/ports.py| src/aiida/manage/configuration/__init__.py| src/aiida/manage/configuration/config.py| - src/aiida/manage/configuration/profile.py| src/aiida/manage/external/rmq/launcher.py| src/aiida/manage/tests/main.py| src/aiida/manage/tests/pytest_fixtures.py| diff --git a/src/aiida/cmdline/commands/cmd_presto.py b/src/aiida/cmdline/commands/cmd_presto.py index 09d7070e7c..eeb98fad75 100644 --- a/src/aiida/cmdline/commands/cmd_presto.py +++ b/src/aiida/cmdline/commands/cmd_presto.py @@ -67,7 +67,7 @@ def detect_postgres_config( """ import secrets - from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER + from aiida.manage.configuration.settings import AiiDAConfigDir from aiida.manage.external.postgres import Postgres dbinfo = { @@ -92,13 +92,15 @@ def detect_postgres_config( except Exception as exception: raise ConnectionError(f'Unable to automatically create the PostgreSQL user and database: {exception}') + aiida_config_folder = AiiDAConfigDir.get() + return { 'database_hostname': postgres_hostname, 'database_port': postgres_port, 'database_name': database_name, 'database_username': database_username, 'database_password': database_password, - 'repository_uri': f'file://{AIIDA_CONFIG_FOLDER / "repository" / profile_name}', + 'repository_uri': f'file://{aiida_config_folder / "repository" / profile_name}', } diff --git a/src/aiida/cmdline/commands/cmd_profile.py b/src/aiida/cmdline/commands/cmd_profile.py index 3dd21b56bf..7cb0e018ae 100644 --- a/src/aiida/cmdline/commands/cmd_profile.py +++ b/src/aiida/cmdline/commands/cmd_profile.py @@ -169,9 +169,9 @@ def profile_list(): # This can happen for a fresh install and the `verdi setup` has not yet been run. In this case it is still nice # to be able to see the configuration directory, for instance for those who have set `AIIDA_PATH`. This way # they can at least verify that it is correctly set. - from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER + from aiida.manage.configuration.settings import AiiDAConfigDir - echo.echo_report(f'configuration folder: {AIIDA_CONFIG_FOLDER}') + echo.echo_report(f'configuration folder: {AiiDAConfigDir.get()}') echo.echo_critical(str(exception)) else: echo.echo_report(f'configuration folder: {config.dirpath}') diff --git a/src/aiida/cmdline/commands/cmd_status.py b/src/aiida/cmdline/commands/cmd_status.py index f3c32327dc..85ef292fa7 100644 --- a/src/aiida/cmdline/commands/cmd_status.py +++ b/src/aiida/cmdline/commands/cmd_status.py @@ -61,13 +61,14 @@ def verdi_status(print_traceback, no_rmq): from aiida.common.docs import URL_NO_BROKER from aiida.common.exceptions import ConfigurationError from aiida.engine.daemon.client import DaemonException, DaemonNotRunningException - from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER + from aiida.manage.configuration.settings import AiiDAConfigDir from aiida.manage.manager import get_manager exit_code = ExitCode.SUCCESS + configure_directory = AiiDAConfigDir.get() print_status(ServiceStatus.UP, 'version', f'AiiDA v{__version__}') - print_status(ServiceStatus.UP, 'config', AIIDA_CONFIG_FOLDER) + print_status(ServiceStatus.UP, 'config', configure_directory) manager = get_manager() diff --git a/src/aiida/cmdline/params/options/commands/setup.py b/src/aiida/cmdline/params/options/commands/setup.py index 930aa97018..49cfc1e121 100644 --- a/src/aiida/cmdline/params/options/commands/setup.py +++ b/src/aiida/cmdline/params/options/commands/setup.py @@ -66,11 +66,12 @@ def get_repository_uri_default(ctx): """ import os - from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER + from aiida.manage.configuration.settings import AiiDAConfigDir validate_profile_parameter(ctx) + configure_directory = AiiDAConfigDir.get() - return os.path.join(AIIDA_CONFIG_FOLDER, 'repository', ctx.params['profile'].name) + return os.path.join(configure_directory, 'repository', ctx.params['profile'].name) def get_quicksetup_repository_uri(ctx, param, value): diff --git a/src/aiida/engine/daemon/client.py b/src/aiida/engine/daemon/client.py index ef250802f7..4e47e7ed60 100644 --- a/src/aiida/engine/daemon/client.py +++ b/src/aiida/engine/daemon/client.py @@ -94,10 +94,10 @@ def __init__(self, profile: Profile): from aiida.common.docs import URL_NO_BROKER type_check(profile, Profile) - config = get_config() + self._config = get_config() self._profile = profile self._socket_directory: str | None = None - self._daemon_timeout: int = config.get_option('daemon.timeout', scope=profile.name) + self._daemon_timeout: int = self._config.get_option('daemon.timeout', scope=profile.name) if self._profile.process_control_backend is None: raise ConfigurationError( @@ -156,31 +156,31 @@ def virtualenv(self) -> str | None: @property def circus_log_file(self) -> str: - return self.profile.filepaths['circus']['log'] + return self._config.filepaths(self.profile)['circus']['log'] @property def circus_pid_file(self) -> str: - return self.profile.filepaths['circus']['pid'] + return self._config.filepaths(self.profile)['circus']['pid'] @property def circus_port_file(self) -> str: - return self.profile.filepaths['circus']['port'] + return self._config.filepaths(self.profile)['circus']['port'] @property def circus_socket_file(self) -> str: - return self.profile.filepaths['circus']['socket']['file'] + return self._config.filepaths(self.profile)['circus']['socket']['file'] @property def circus_socket_endpoints(self) -> dict[str, str]: - return self.profile.filepaths['circus']['socket'] + return self._config.filepaths(self.profile)['circus']['socket'] @property def daemon_log_file(self) -> str: - return self.profile.filepaths['daemon']['log'] + return self._config.filepaths(self.profile)['daemon']['log'] @property def daemon_pid_file(self) -> str: - return self.profile.filepaths['daemon']['pid'] + return self._config.filepaths(self.profile)['daemon']['pid'] def get_circus_port(self) -> int: """Retrieve the port for the circus controller, which should be written to the circus port file. diff --git a/src/aiida/manage/configuration/__init__.py b/src/aiida/manage/configuration/__init__.py index 7227281507..097fcfc012 100644 --- a/src/aiida/manage/configuration/__init__.py +++ b/src/aiida/manage/configuration/__init__.py @@ -65,10 +65,10 @@ def get_config_path(): - """Returns path to .aiida configuration directory.""" - from .settings import AIIDA_CONFIG_FOLDER, DEFAULT_CONFIG_FILE_NAME + """Returns path to aiida configuration file.""" + from .settings import DEFAULT_CONFIG_FILE_NAME, AiiDAConfigDir - return os.path.join(AIIDA_CONFIG_FOLDER, DEFAULT_CONFIG_FILE_NAME) + return os.path.join(AiiDAConfigDir.get(), DEFAULT_CONFIG_FILE_NAME) def load_config(create=False) -> 'Config': diff --git a/src/aiida/manage/configuration/config.py b/src/aiida/manage/configuration/config.py index 4b1f032271..fff83f4330 100644 --- a/src/aiida/manage/configuration/config.py +++ b/src/aiida/manage/configuration/config.py @@ -21,6 +21,7 @@ import json import os import uuid +from pathlib import Path from typing import Any, Dict, List, Optional, Tuple from pydantic import ( @@ -780,3 +781,32 @@ def _atomic_write(self, filepath=None): handle.flush() os.rename(handle.name, self.filepath) + + def filepaths(self, profile: Profile): + """Return the filepaths used by a profile. + + :return: a dictionary of filepaths + """ + from aiida.manage.configuration.settings import AiiDAConfigPathResolver + + _config_path_resolver: AiiDAConfigPathResolver = AiiDAConfigPathResolver(Path(self.dirpath)) + daemon_dir = _config_path_resolver.daemon_dir + daemon_log_dir = _config_path_resolver.daemon_log_dir + + return { + 'circus': { + 'log': str(daemon_log_dir / f'circus-{profile.name}.log'), + 'pid': str(daemon_dir / f'circus-{profile.name}.pid'), + 'port': str(daemon_dir / f'circus-{profile.name}.port'), + 'socket': { + 'file': str(daemon_dir / f'circus-{profile.name}.sockets'), + 'controller': 'circus.c.sock', + 'pubsub': 'circus.p.sock', + 'stats': 'circus.s.sock', + }, + }, + 'daemon': { + 'log': str(daemon_log_dir / f'aiida-{profile.name}.log'), + 'pid': str(daemon_dir / f'aiida-{profile.name}.pid'), + }, + } diff --git a/src/aiida/manage/configuration/profile.py b/src/aiida/manage/configuration/profile.py index acaca2e892..93a3c5c911 100644 --- a/src/aiida/manage/configuration/profile.py +++ b/src/aiida/manage/configuration/profile.py @@ -56,7 +56,7 @@ def __init__(self, name: str, config: Mapping[str, Any], validate=True): ) self._name = name - self._attributes: Dict[str, Any] = deepcopy(config) + self._attributes: Dict[str, Any] = deepcopy(config) # type: ignore[arg-type] # Create a default UUID if not specified if self._attributes.get(self.KEY_UUID, None) is None: @@ -235,22 +235,28 @@ def filepaths(self): :return: a dictionary of filepaths """ - from .settings import DAEMON_DIR, DAEMON_LOG_DIR + from aiida.common.warnings import warn_deprecation + from aiida.manage.configuration.settings import AiiDAConfigPathResolver + + warn_deprecation('This method has been deprecated, use `filepaths` method from `Config` obj instead', version=3) + + daemon_dir = AiiDAConfigPathResolver().daemon_dir + daemon_log_dir = AiiDAConfigPathResolver().daemon_log_dir return { 'circus': { - 'log': str(DAEMON_LOG_DIR / f'circus-{self.name}.log'), - 'pid': str(DAEMON_DIR / f'circus-{self.name}.pid'), - 'port': str(DAEMON_DIR / f'circus-{self.name}.port'), + 'log': str(daemon_log_dir / f'circus-{self.name}.log'), + 'pid': str(daemon_dir / f'circus-{self.name}.pid'), + 'port': str(daemon_dir / f'circus-{self.name}.port'), 'socket': { - 'file': str(DAEMON_DIR / f'circus-{self.name}.sockets'), + 'file': str(daemon_dir / f'circus-{self.name}.sockets'), 'controller': 'circus.c.sock', 'pubsub': 'circus.p.sock', 'stats': 'circus.s.sock', }, }, 'daemon': { - 'log': str(DAEMON_LOG_DIR / f'aiida-{self.name}.log'), - 'pid': str(DAEMON_DIR / f'aiida-{self.name}.pid'), + 'log': str(daemon_log_dir / f'aiida-{self.name}.log'), + 'pid': str(daemon_dir / f'aiida-{self.name}.pid'), }, } diff --git a/src/aiida/manage/configuration/settings.py b/src/aiida/manage/configuration/settings.py index 168abb8879..f47a2dd66e 100644 --- a/src/aiida/manage/configuration/settings.py +++ b/src/aiida/manage/configuration/settings.py @@ -13,6 +13,7 @@ import os import pathlib import warnings +from typing import final DEFAULT_UMASK = 0o0077 DEFAULT_AIIDA_PATH_VARIABLE = 'AIIDA_PATH' @@ -25,38 +26,86 @@ DEFAULT_DAEMON_LOG_DIR_NAME = 'log' DEFAULT_ACCESS_CONTROL_DIR_NAME = 'access' -# Assign defaults which may be overriden in set_configuration_directory() below -AIIDA_CONFIG_FOLDER: pathlib.Path = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME -DAEMON_DIR: pathlib.Path = AIIDA_CONFIG_FOLDER / DEFAULT_DAEMON_DIR_NAME -DAEMON_LOG_DIR: pathlib.Path = DAEMON_DIR / DEFAULT_DAEMON_LOG_DIR_NAME -ACCESS_CONTROL_DIR: pathlib.Path = AIIDA_CONFIG_FOLDER / DEFAULT_ACCESS_CONTROL_DIR_NAME +__all__ = ('AiiDAConfigPathResolver', 'AiiDAConfigDir') -def create_instance_directories() -> None: +@final +class AiiDAConfigDir: + """Singleton for setting and getting the path to configuration directory.""" + + _glb_aiida_config_folder: pathlib.Path = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME + + @classmethod + def get(cls): + """Return the path of the configuration directory.""" + return cls._glb_aiida_config_folder + + @classmethod + def set(cls, aiida_config_folder: pathlib.Path | None = None) -> None: + """Set the configuration directory, related global variables and create instance directories. + + The location of the configuration directory is defined by ``aiida_config_folder`` or if not defined, + the path that is returned by ``_get_configuration_directory_from_envvar``. + Or if the environment_variable not set, use the default. + If the directory does not exist yet, it is created, together with all its subdirectories. + """ + _default_dirpath_config = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME + + aiida_config_folder = aiida_config_folder or _get_configuration_directory_from_envvar() + cls._glb_aiida_config_folder = aiida_config_folder or _default_dirpath_config + + _create_instance_directories(cls._glb_aiida_config_folder) + + +@final +class AiiDAConfigPathResolver: + """For resolving configuration directory, daemon dir, daemon log dir and access control dir. + The locations are all trivially derived from the config directory. + """ + + def __init__(self, config_folder: pathlib.Path | None = None) -> None: + self._aiida_path = config_folder or AiiDAConfigDir.get() + + @property + def aiida_path(self) -> pathlib.Path: + return self._aiida_path + + @property + def daemon_dir(self) -> pathlib.Path: + return self._aiida_path / DEFAULT_DAEMON_DIR_NAME + + @property + def daemon_log_dir(self) -> pathlib.Path: + return self._aiida_path / DEFAULT_DAEMON_DIR_NAME / DEFAULT_DAEMON_LOG_DIR_NAME + + @property + def access_control_dir(self) -> pathlib.Path: + return self._aiida_path / DEFAULT_ACCESS_CONTROL_DIR_NAME + + +def _create_instance_directories(aiida_config_folder: pathlib.Path | None) -> None: """Create the base directories required for a new AiiDA instance. - This will create the base AiiDA directory defined by the AIIDA_CONFIG_FOLDER variable, unless it already exists. - Subsequently, it will create the daemon directory within it and the daemon log directory. + This will create the base AiiDA directory in ``aiida_config_folder``. + If it not provided, the directory returned from ``AiiDAConfigDir.get()`` will be the default config folder, + unless it already exists. Subsequently, it will create the daemon directory within it and the daemon log directory. """ from aiida.common import ConfigurationError - directory_base = AIIDA_CONFIG_FOLDER.expanduser() - directory_daemon = directory_base / DAEMON_DIR - directory_daemon_log = directory_base / DAEMON_LOG_DIR - directory_access = directory_base / ACCESS_CONTROL_DIR + path_resolver = AiiDAConfigPathResolver(aiida_config_folder) list_of_paths = [ - directory_base, - directory_daemon, - directory_daemon_log, - directory_access, + path_resolver.aiida_path, + path_resolver.daemon_dir, + path_resolver.daemon_log_dir, + path_resolver.access_control_dir, ] umask = os.umask(DEFAULT_UMASK) try: for path in list_of_paths: - if path is directory_base and not path.exists(): + if path is path_resolver.aiida_path and not path.exists(): warnings.warn(f'Creating AiiDA configuration folder `{path}`.') try: @@ -64,31 +113,10 @@ def create_instance_directories() -> None: except OSError as exc: raise ConfigurationError(f'could not create the `{path}` configuration directory: {exc}') from exc finally: - os.umask(umask) + _ = os.umask(umask) -def get_configuration_directory(): - """Return the path of the configuration directory. - - The location of the configuration directory is defined following these heuristics in order: - - * If the ``AIIDA_PATH`` variable is set, all the paths will be checked to see if they contain a - configuration folder. The first one to be encountered will be set as ``AIIDA_CONFIG_FOLDER``. If none of them - contain one, the last path defined in the environment variable considered is used. - * If an existing directory is still not found, the ``DEFAULT_AIIDA_PATH`` is used. - - :returns: The path of the configuration directory. - """ - dirpath_config = get_configuration_directory_from_envvar() - - # If no existing configuration directory is found, fall back to the default - if dirpath_config is None: - dirpath_config = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME - - return dirpath_config - - -def get_configuration_directory_from_envvar() -> pathlib.Path | None: +def _get_configuration_directory_from_envvar() -> pathlib.Path | None: """Return the path of a config directory from the ``AIIDA_PATH`` environment variable. The environment variable should be a colon separated string of filepaths that either point directly to a config @@ -99,10 +127,13 @@ def get_configuration_directory_from_envvar() -> pathlib.Path | None: """ environment_variable = os.environ.get(DEFAULT_AIIDA_PATH_VARIABLE) + default_dirpath_config = pathlib.Path(DEFAULT_AIIDA_PATH).expanduser() / DEFAULT_CONFIG_DIR_NAME + if environment_variable is None: return None # Loop over all the paths in the ``AIIDA_PATH`` variable to see if any of them contain a configuration folder + dirpath_config = None for base_dir_path in [path for path in environment_variable.split(':') if path]: dirpath_config = pathlib.Path(base_dir_path).expanduser() @@ -115,28 +146,8 @@ def get_configuration_directory_from_envvar() -> pathlib.Path | None: if dirpath_config.is_dir(): break - return dirpath_config - - -def set_configuration_directory(aiida_config_folder: pathlib.Path | None = None) -> None: - """Set the configuration directory, related global variables and create instance directories. - - The location of the configuration directory is defined by ``aiida_config_folder`` or if not defined, the path that - is returned by ``get_configuration_directory``. If the directory does not exist yet, it is created, together with - all its subdirectories. - """ - global AIIDA_CONFIG_FOLDER # noqa: PLW0603 - global DAEMON_DIR # noqa: PLW0603 - global DAEMON_LOG_DIR # noqa: PLW0603 - global ACCESS_CONTROL_DIR # noqa: PLW0603 - - AIIDA_CONFIG_FOLDER = aiida_config_folder or get_configuration_directory() - DAEMON_DIR = AIIDA_CONFIG_FOLDER / DEFAULT_DAEMON_DIR_NAME - DAEMON_LOG_DIR = DAEMON_DIR / DEFAULT_DAEMON_LOG_DIR_NAME - ACCESS_CONTROL_DIR = AIIDA_CONFIG_FOLDER / DEFAULT_ACCESS_CONTROL_DIR_NAME - - create_instance_directories() + return dirpath_config or default_dirpath_config # Initialize the configuration directory settings -set_configuration_directory() +AiiDAConfigDir.set() diff --git a/src/aiida/manage/profile_access.py b/src/aiida/manage/profile_access.py index 5b04481e66..c65364af03 100644 --- a/src/aiida/manage/profile_access.py +++ b/src/aiida/manage/profile_access.py @@ -18,8 +18,10 @@ from aiida.common.exceptions import LockedProfileError, LockingProfileError from aiida.common.lang import type_check from aiida.manage.configuration import Profile +from aiida.manage.configuration.settings import AiiDAConfigPathResolver +@typing.final class ProfileAccessManager: """Class to manage access to a profile. @@ -45,12 +47,10 @@ def __init__(self, profile: Profile): :param profile: the profile whose access to manage. """ - from aiida.manage.configuration.settings import ACCESS_CONTROL_DIR - - type_check(profile, Profile) + _ = type_check(profile, Profile) self.profile = profile self.process = psutil.Process(os.getpid()) - self._dirpath_records = ACCESS_CONTROL_DIR / profile.name + self._dirpath_records = AiiDAConfigPathResolver().access_control_dir / profile.name self._dirpath_records.mkdir(exist_ok=True) def request_access(self) -> None: diff --git a/src/aiida/manage/tests/pytest_fixtures.py b/src/aiida/manage/tests/pytest_fixtures.py index 92856aff66..6c5d04d3bc 100644 --- a/src/aiida/manage/tests/pytest_fixtures.py +++ b/src/aiida/manage/tests/pytest_fixtures.py @@ -162,6 +162,7 @@ def aiida_instance( """ from aiida.manage import configuration from aiida.manage.configuration import settings + from aiida.manage.configuration.settings import AiiDAConfigDir if aiida_test_profile: yield configuration.get_config() @@ -178,8 +179,7 @@ def aiida_instance( dirpath_config = tmp_path_factory.mktemp('config') os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = str(dirpath_config) - settings.AIIDA_CONFIG_FOLDER = dirpath_config - settings.set_configuration_directory() + AiiDAConfigDir.set(dirpath_config) configuration.CONFIG = configuration.load_config(create=True) try: @@ -191,7 +191,7 @@ def aiida_instance( else: os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = current_path_variable - settings.AIIDA_CONFIG_FOLDER = current_config_path + AiiDAConfigDir.set(current_config_path) configuration.CONFIG = current_config if current_profile: aiida_manager.load_profile(current_profile.name, allow_switch=True) diff --git a/src/aiida/storage/sqlite_dos/backend.py b/src/aiida/storage/sqlite_dos/backend.py index 7be70f4a1c..2443fee259 100644 --- a/src/aiida/storage/sqlite_dos/backend.py +++ b/src/aiida/storage/sqlite_dos/backend.py @@ -26,7 +26,7 @@ from aiida.common import exceptions from aiida.common.log import AIIDA_LOGGER from aiida.manage.configuration.profile import Profile -from aiida.manage.configuration.settings import AIIDA_CONFIG_FOLDER +from aiida.manage.configuration.settings import AiiDAConfigDir from aiida.orm.implementation import BackendEntity from aiida.storage.log import MIGRATE_LOGGER from aiida.storage.psql_dos.models.settings import DbSetting @@ -203,7 +203,7 @@ class Model(BaseModel, defer_build=True): filepath: str = Field( title='Directory of the backend', description='Filepath of the directory in which to store data for this backend.', - default_factory=lambda: str(AIIDA_CONFIG_FOLDER / 'repository' / f'sqlite_dos_{uuid4().hex}'), + default_factory=lambda: str(AiiDAConfigDir.get() / 'repository' / f'sqlite_dos_{uuid4().hex}'), ) @field_validator('filepath') diff --git a/src/aiida/tools/pytest_fixtures/configuration.py b/src/aiida/tools/pytest_fixtures/configuration.py index ca38db3fb0..ed06072b71 100644 --- a/src/aiida/tools/pytest_fixtures/configuration.py +++ b/src/aiida/tools/pytest_fixtures/configuration.py @@ -10,6 +10,8 @@ import pytest +from aiida.manage.configuration.settings import AiiDAConfigDir + if t.TYPE_CHECKING: from aiida.manage.configuration.config import Config @@ -53,7 +55,7 @@ def factory(dirpath: pathlib.Path): dirpath_config = dirpath / settings.DEFAULT_CONFIG_DIR_NAME os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = str(dirpath_config) - settings.set_configuration_directory(dirpath_config) + AiiDAConfigDir.set(dirpath_config) config = get_config(create=True) try: @@ -61,7 +63,7 @@ def factory(dirpath: pathlib.Path): finally: if current_config: reset_config() - settings.set_configuration_directory(pathlib.Path(current_config.dirpath)) + AiiDAConfigDir.set(pathlib.Path(current_config.dirpath)) get_config() if current_path_variable is None: diff --git a/src/aiida/transports/plugins/local.py b/src/aiida/transports/plugins/local.py index 755476a066..8de49838e3 100644 --- a/src/aiida/transports/plugins/local.py +++ b/src/aiida/transports/plugins/local.py @@ -453,8 +453,8 @@ def getfile(self, remotepath, localpath, *args, **kwargs): """Copies a file recursively from 'remote' remotepath to 'local' localpath. - :param remotepath: path to local file - :param localpath: absolute path to remote file + :param remotepath: absolute path to remote file + :param localpath: path to local file :param overwrite: if True overwrites localpath. Default = False @@ -462,6 +462,9 @@ def getfile(self, remotepath, localpath, *args, **kwargs): :raise ValueError: if 'local' localpath is not valid :raise OSError: if unintentionally overwriting """ + if not os.path.isabs(localpath): + raise ValueError('localpath must be an absolute path') + overwrite = kwargs.get('overwrite', args[0] if args else True) if not localpath: raise ValueError('Input localpath to get function must be a non empty string') diff --git a/tests/conftest.py b/tests/conftest.py index 5b5bedad02..89b0a1bad7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -374,7 +374,7 @@ def empty_config(tmp_path) -> Config: """ from aiida.common.utils import Capturing from aiida.manage import configuration, get_manager - from aiida.manage.configuration import settings + from aiida.manage.configuration.settings import AiiDAConfigDir manager = get_manager() @@ -388,7 +388,7 @@ def empty_config(tmp_path) -> Config: # Set the configuration directory to a temporary directory. This will create the necessary folders for an empty # AiiDA configuration and set relevant global variables in :mod:`aiida.manage.configuration.settings`. - settings.set_configuration_directory(tmp_path) + AiiDAConfigDir.set(tmp_path) # The constructor of `Config` called by `load_config` will print warning messages about migrating it with Capturing(): @@ -406,7 +406,7 @@ def empty_config(tmp_path) -> Config: # like the :class:`aiida.engine.daemon.client.DaemonClient` will not function properly after a test that uses # this fixture because the paths of the daemon files would still point to the path of the temporary config # folder created by this fixture. - settings.set_configuration_directory(pathlib.Path(current_config_path)) + AiiDAConfigDir.set(pathlib.Path(current_config_path)) # Reload the original profile manager.load_profile(current_profile_name) diff --git a/tests/manage/configuration/test_config.py b/tests/manage/configuration/test_config.py index 5f24a5690a..77adc4bf6d 100644 --- a/tests/manage/configuration/test_config.py +++ b/tests/manage/configuration/test_config.py @@ -20,6 +20,7 @@ from aiida.manage.configuration.config import Config from aiida.manage.configuration.migrations import CURRENT_CONFIG_VERSION, OLDEST_COMPATIBLE_CONFIG_VERSION from aiida.manage.configuration.options import get_option +from aiida.manage.configuration.settings import AiiDAConfigDir from aiida.storage.sqlite_temp import SqliteTempBackend @@ -43,7 +44,7 @@ def cache_aiida_path_variable(): # Make sure to reset the global variables set by the following call that are dependent on the environment variable # ``DEFAULT_AIIDA_PATH_VARIABLE``. It may have been changed by a test using this fixture. - settings.set_configuration_directory() + AiiDAConfigDir.set() @pytest.mark.filterwarnings('ignore:Creating AiiDA configuration folder') @@ -66,11 +67,11 @@ def test_environment_variable_not_set(chdir_tmp_path, monkeypatch): del os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] except KeyError: pass - settings.set_configuration_directory() + AiiDAConfigDir.set() config_folder = chdir_tmp_path / settings.DEFAULT_CONFIG_DIR_NAME assert os.path.isdir(config_folder) - assert settings.AIIDA_CONFIG_FOLDER == pathlib.Path(config_folder) + assert AiiDAConfigDir.get() == pathlib.Path(config_folder) @pytest.mark.filterwarnings('ignore:Creating AiiDA configuration folder') @@ -79,12 +80,12 @@ def test_environment_variable_set_single_path_without_config_folder(tmp_path): """If `AIIDA_PATH` is set but does not contain a configuration folder, it should be created.""" # Set the environment variable and call configuration initialization os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = str(tmp_path) - settings.set_configuration_directory() + AiiDAConfigDir.set() # This should have created the configuration directory in the path config_folder = tmp_path / settings.DEFAULT_CONFIG_DIR_NAME assert config_folder.is_dir() - assert settings.AIIDA_CONFIG_FOLDER == config_folder + assert AiiDAConfigDir.get() == config_folder @pytest.mark.filterwarnings('ignore:Creating AiiDA configuration folder') @@ -95,12 +96,12 @@ def test_environment_variable_set_single_path_with_config_folder(tmp_path): # Set the environment variable and call configuration initialization os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = str(tmp_path) - settings.set_configuration_directory() + AiiDAConfigDir.set() # This should have created the configuration directory in the path config_folder = tmp_path / settings.DEFAULT_CONFIG_DIR_NAME assert config_folder.is_dir() - assert settings.AIIDA_CONFIG_FOLDER == config_folder + assert AiiDAConfigDir.get() == config_folder @pytest.mark.filterwarnings('ignore:Creating AiiDA configuration folder') @@ -115,12 +116,12 @@ def test_environment_variable_path_including_config_folder(tmp_path): """ # Set the environment variable with a path that include base folder name and call config initialization os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = str(tmp_path / settings.DEFAULT_CONFIG_DIR_NAME) - settings.set_configuration_directory() + AiiDAConfigDir.set() # This should have created the configuration directory in the pathpath config_folder = tmp_path / settings.DEFAULT_CONFIG_DIR_NAME assert config_folder.is_dir() - assert settings.AIIDA_CONFIG_FOLDER == config_folder + assert AiiDAConfigDir.get() == config_folder @pytest.mark.filterwarnings('ignore:Creating AiiDA configuration folder') @@ -138,12 +139,12 @@ def test_environment_variable_set_multiple_path(tmp_path): # Set the environment variable to contain three paths and call configuration initialization env_variable = f'{directory_a}:{directory_b}:{directory_c}' os.environ[settings.DEFAULT_AIIDA_PATH_VARIABLE] = env_variable - settings.set_configuration_directory() + AiiDAConfigDir.set() # This should have created the configuration directory in the last path config_folder = directory_c / settings.DEFAULT_CONFIG_DIR_NAME assert os.path.isdir(config_folder) - assert settings.AIIDA_CONFIG_FOLDER == config_folder + assert AiiDAConfigDir.get() == config_folder def compare_config_in_memory_and_on_disk(config, filepath): @@ -153,9 +154,7 @@ def compare_config_in_memory_and_on_disk(config, filepath): :param filepath: absolute filepath to a configuration file :raises AssertionError: if content of `config` is not equal to that of file on disk """ - from aiida.manage.configuration.settings import DEFAULT_CONFIG_INDENT_SIZE - - in_memory = json.dumps(config.dictionary, indent=DEFAULT_CONFIG_INDENT_SIZE) + in_memory = json.dumps(config.dictionary, indent=settings.DEFAULT_CONFIG_INDENT_SIZE) # Read the content stored on disk with open(filepath, 'r', encoding='utf8') as handle: diff --git a/tests/manage/test_caching_config.py b/tests/manage/test_caching_config.py index 5521ea8288..1138cf393d 100644 --- a/tests/manage/test_caching_config.py +++ b/tests/manage/test_caching_config.py @@ -46,11 +46,12 @@ def test_merge_deprecated_yaml(tmp_path): """ from aiida.common.warnings import AiidaDeprecationWarning from aiida.manage import configuration, get_manager - from aiida.manage.configuration import get_config_option, load_profile, settings + from aiida.manage.configuration import get_config_option, load_profile + from aiida.manage.configuration.settings import AiiDAConfigDir # Store the current configuration instance and config directory path current_config = configuration.CONFIG - current_config_path = current_config.dirpath + current_config_path = pathlib.Path(current_config.dirpath) current_profile_name = configuration.get_profile().name try: @@ -58,7 +59,7 @@ def test_merge_deprecated_yaml(tmp_path): configuration.CONFIG = None # Create a temporary folder, set it as the current config directory path - settings.AIIDA_CONFIG_FOLDER = str(tmp_path) + AiiDAConfigDir.set(pathlib.Path(tmp_path)) config_dictionary = json.loads( pathlib.Path(__file__) .parent.joinpath('configuration/migrations/test_samples/reference/6.json') @@ -87,7 +88,7 @@ def test_merge_deprecated_yaml(tmp_path): # Reset the config folder path and the config instance. Note this will always be executed after the yield no # matter what happened in the test that used this fixture. get_manager().unload_profile() - settings.AIIDA_CONFIG_FOLDER = current_config_path + AiiDAConfigDir.set(current_config_path) configuration.CONFIG = current_config load_profile(current_profile_name) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 918f4a051c..7b4fa6954f 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -12,15 +12,12 @@ """ import io -import os -import pathlib -import random import shutil import signal -import string import tempfile import time import uuid +from pathlib import Path import psutil import pytest @@ -34,8 +31,20 @@ # TODO : silly cases of copy/put/get from self to self +@pytest.fixture(scope='function') +def tmp_path_remote(tmp_path_factory): + """Mock the remote tmp path using tmp_path_factory to create folder start with prefix 'remote'""" + return tmp_path_factory.mktemp('remote') + + +@pytest.fixture(scope='function') +def tmp_path_local(tmp_path_factory): + """Mock the local tmp path using tmp_path_factory to create folder start with prefix 'local'""" + return tmp_path_factory.mktemp('local') + + @pytest.fixture(scope='function', params=entry_point.get_entry_point_names('aiida.transports')) -def custom_transport(request, tmp_path, monkeypatch) -> Transport: +def custom_transport(request, tmp_path_factory, monkeypatch) -> Transport: """Fixture that parametrizes over all the registered implementations of the ``CommonRelaxWorkChain``.""" plugin = TransportFactory(request.param) @@ -43,10 +52,12 @@ def custom_transport(request, tmp_path, monkeypatch) -> Transport: kwargs = {'machine': 'localhost', 'timeout': 30, 'load_system_host_keys': True, 'key_policy': 'AutoAddPolicy'} elif request.param == 'core.ssh_auto': kwargs = {'machine': 'localhost'} - filepath_config = tmp_path / 'config' + # The transport config is store in a independent temporary path per test to not mix up + # with the files under operating. + filepath_config = tmp_path_factory.mktemp('transport') / 'config' monkeypatch.setattr(plugin, 'FILEPATH_CONFIG', filepath_config) - if not filepath_config.exists(): - filepath_config.write_text('Host localhost') + + filepath_config.write_text('Host localhost') else: kwargs = {} @@ -63,26 +74,36 @@ def test_is_open(custom_transport): assert not custom_transport.is_open -def test_makedirs(custom_transport): - """Verify the functioning of makedirs command""" +def test_chdir_and_getcwd_deprecated(custom_transport, tmp_path_remote): + """Test to be deprecated ``chdir``/``getcwd`` methods still work.""" with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' + location = str(tmp_path_remote) transport.chdir(location) assert location == transport.getcwd() - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) - transport.chdir(directory) + +def test_chdir_to_empty_string_deprecated(custom_transport, tmp_path_remote): + """I check that if I pass an empty string to chdir, the cwd does + not change (this is a paramiko default behavior), but getcwd() + is still correctly defined. + """ + with custom_transport as transport: + new_dir = str(tmp_path_remote) + transport.chdir(new_dir) + transport.chdir('') + assert new_dir == transport.getcwd() + + +def test_makedirs(custom_transport, tmp_path_remote): + """Verify the functioning of makedirs command""" + with custom_transport as transport: # define folder structure - dir_tree = os.path.join('1', '2') + dir_tree = str(tmp_path_remote / '1' / '2') # I create the tree transport.makedirs(dir_tree) # verify the existence - assert transport.isdir('1') + assert transport.isdir(str(tmp_path_remote / '1')) assert dir_tree # try to recreate the same folder @@ -92,93 +113,55 @@ def test_makedirs(custom_transport): # recreate but with ignore flag transport.makedirs(dir_tree, True) - transport.rmdir(dir_tree) - transport.rmdir('1') - - transport.chdir('..') - transport.rmdir(directory) - -def test_rmtree(custom_transport): +def test_rmtree(custom_transport, tmp_path_remote): """Verify the functioning of rmtree command""" with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - assert location == transport.getcwd() - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) - transport.chdir(directory) - # define folder structure - dir_tree = os.path.join('1', '2') + dir_tree = str(tmp_path_remote / '1' / '2') # I create the tree transport.makedirs(dir_tree) # remove it - transport.rmtree('1') + transport.rmtree(str(tmp_path_remote / '1')) # verify the removal - assert not transport.isdir('1') + assert not transport.isdir(str(tmp_path_remote / '1')) # also tests that it works with a single file # create file local_file_name = 'file.txt' text = 'Viva Verdi\n' - with open(os.path.join(transport.getcwd(), local_file_name), 'w', encoding='utf8') as fhandle: + single_file_path = str(tmp_path_remote / local_file_name) + with open(single_file_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) # remove it - transport.rmtree(local_file_name) + transport.rmtree(single_file_path) # verify the removal - assert not transport.isfile(local_file_name) - - transport.chdir('..') - transport.rmdir(directory) + assert not transport.isfile(single_file_path) -def test_listdir(custom_transport): +def test_listdir(custom_transport, tmp_path_remote): """Create directories, verify listdir, delete a folder with subfolders""" - with custom_transport as trans: - # We cannot use tempfile.mkdtemp because we're on a remote folder - location = trans.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - trans.chdir(location) - - assert location == trans.getcwd() - while trans.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - trans.mkdir(directory) - trans.chdir(directory) + with custom_transport as transport: list_of_dir = ['1', '-f a&', 'as', 'a2', 'a4f'] list_of_files = ['a', 'b'] for this_dir in list_of_dir: - trans.mkdir(this_dir) + transport.mkdir(str(tmp_path_remote / this_dir)) + for fname in list_of_files: with tempfile.NamedTemporaryFile() as tmpf: # Just put an empty file there at the right file name - trans.putfile(tmpf.name, fname) + transport.putfile(tmpf.name, str(tmp_path_remote / fname)) - list_found = trans.listdir('.') + list_found = transport.listdir(str(tmp_path_remote)) assert sorted(list_found) == sorted(list_of_dir + list_of_files) - assert sorted(trans.listdir('.', 'a*')), sorted(['as', 'a2', 'a4f']) - assert sorted(trans.listdir('.', 'a?')), sorted(['as', 'a2']) - assert sorted(trans.listdir('.', 'a[2-4]*')), sorted(['a2', 'a4f']) - - for this_dir in list_of_dir: - trans.rmdir(this_dir) - - for this_file in list_of_files: - trans.remove(this_file) - - trans.chdir('..') - trans.rmdir(directory) + assert sorted(transport.listdir(str(tmp_path_remote), 'a*')), sorted(['as', 'a2', 'a4f']) + assert sorted(transport.listdir(str(tmp_path_remote), 'a?')), sorted(['as', 'a2']) + assert sorted(transport.listdir(str(tmp_path_remote), 'a[2-4]*')), sorted(['a2', 'a4f']) -def test_listdir_withattributes(custom_transport): +def test_listdir_withattributes(custom_transport, tmp_path_remote): """Create directories, verify listdir_withattributes, delete a folder with subfolders""" def simplify_attributes(data): @@ -190,115 +173,79 @@ def simplify_attributes(data): """ return {_['name']: _['isdir'] for _ in data} - with custom_transport as trans: - # We cannot use tempfile.mkdtemp because we're on a remote folder - location = trans.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - trans.chdir(location) - - assert location == trans.getcwd() - while trans.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - trans.mkdir(directory) - trans.chdir(directory) + with custom_transport as transport: list_of_dir = ['1', '-f a&', 'as', 'a2', 'a4f'] list_of_files = ['a', 'b'] for this_dir in list_of_dir: - trans.mkdir(this_dir) + transport.mkdir(str(tmp_path_remote / this_dir)) + for fname in list_of_files: with tempfile.NamedTemporaryFile() as tmpf: # Just put an empty file there at the right file name - trans.putfile(tmpf.name, fname) + transport.putfile(tmpf.name, str(tmp_path_remote / fname)) comparison_list = {k: True for k in list_of_dir} for k in list_of_files: comparison_list[k] = False - assert simplify_attributes(trans.listdir_withattributes('.')), comparison_list - assert simplify_attributes(trans.listdir_withattributes('.', 'a*')), { + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote))), comparison_list + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote), 'a*')), { 'as': True, 'a2': True, 'a4f': True, 'a': False, } - assert simplify_attributes(trans.listdir_withattributes('.', 'a?')), {'as': True, 'a2': True} - assert simplify_attributes(trans.listdir_withattributes('.', 'a[2-4]*')), {'a2': True, 'a4f': True} - - for this_dir in list_of_dir: - trans.rmdir(this_dir) - - for this_file in list_of_files: - trans.remove(this_file) - - trans.chdir('..') - trans.rmdir(directory) + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote), 'a?')), { + 'as': True, + 'a2': True, + } + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote), 'a[2-4]*')), { + 'a2': True, + 'a4f': True, + } -def test_dir_creation_deletion(custom_transport): +def test_dir_creation_deletion(custom_transport, tmp_path_remote): """Test creating and deleting directories.""" with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - assert location == transport.getcwd() - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) + new_dir = str(tmp_path_remote / 'new') + transport.mkdir(new_dir) with pytest.raises(OSError): # I create twice the same directory - transport.mkdir(directory) + transport.mkdir(new_dir) - transport.isdir(directory) - assert not transport.isfile(directory) - transport.rmdir(directory) + transport.isdir(new_dir) + assert not transport.isfile(new_dir) -def test_dir_copy(custom_transport): +def test_dir_copy(custom_transport, tmp_path_remote): """Verify if in the copy of a directory also the protection bits are carried over """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) + # Create a src dir + src_dir = str(tmp_path_remote / 'copy_src') + transport.mkdir(src_dir) - dest_directory = f'{directory}_copy' - transport.copy(directory, dest_directory) + dst_dir = str(tmp_path_remote / 'copy_dst') + transport.copy(src_dir, dst_dir) with pytest.raises(ValueError): - transport.copy(directory, '') + transport.copy(src_dir, '') with pytest.raises(ValueError): - transport.copy('', directory) + transport.copy('', dst_dir) - transport.rmdir(directory) - transport.rmdir(dest_directory) - -def test_dir_permissions_creation_modification(custom_transport): +def test_dir_permissions_creation_modification(custom_transport, tmp_path_remote): """Verify if chmod raises OSError when trying to change bits on a non-existing folder """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) + directory = str(tmp_path_remote / 'test') - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - - # create directory with non default permissions - transport.mkdir(directory) + transport.makedirs(directory) # change permissions transport.chmod(directory, 0o777) @@ -319,7 +266,6 @@ def test_dir_permissions_creation_modification(custom_transport): # the new directory modes. To see if we want a higher # level function to ask for the mode, or we just # use get_attribute - transport.chdir(directory) # change permissions of an empty string, non existing folder. fake_dir = '' @@ -329,24 +275,15 @@ def test_dir_permissions_creation_modification(custom_transport): fake_dir = 'pippo' with pytest.raises(OSError): # chmod to a non existing folder - transport.chmod(fake_dir, 0o777) + transport.chmod(str(tmp_path_remote / fake_dir), 0o777) - transport.chdir('..') - transport.rmdir(directory) - -def test_dir_reading_permissions(custom_transport): +def test_dir_reading_permissions(custom_transport, tmp_path_remote): """Try to enter a directory with no read permissions. Verify that the cwd has not changed after failed try. """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + directory = str(tmp_path_remote / 'test') # create directory with non default permissions transport.mkdir(directory) @@ -366,278 +303,250 @@ def test_dir_reading_permissions(custom_transport): assert old_cwd == new_cwd - # TODO : the test leaves a directory even if it is successful - # The bug is in paramiko. After lowering the permissions, - # I cannot restore them to higher values - # transport.rmdir(directory) - def test_isfile_isdir_to_empty_string(custom_transport): """I check that isdir or isfile return False when executed on an empty string """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - transport.chdir(location) assert not transport.isdir('') assert not transport.isfile('') -def test_isfile_isdir_to_non_existing_string(custom_transport): +def test_isfile_isdir_to_non_existing_string(custom_transport, tmp_path_remote): """I check that isdir or isfile return False when executed on an empty string """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - transport.chdir(location) - fake_folder = 'pippo' + fake_folder = str(tmp_path_remote / 'pippo') assert not transport.isfile(fake_folder) assert not transport.isdir(fake_folder) with pytest.raises(OSError): transport.chdir(fake_folder) -def test_chdir_to_empty_string(custom_transport): - """I check that if I pass an empty string to chdir, the cwd does - not change (this is a paramiko default behavior), but getcwd() - is still correctly defined. - """ +def test_put_and_get(custom_transport, tmp_path_remote, tmp_path_local): + """Test putting and getting files.""" + directory = 'tmp_try' + with custom_transport as transport: - new_dir = transport.normalize(os.path.join('/', 'tmp')) - transport.chdir(new_dir) - transport.chdir('') - assert new_dir == transport.getcwd() + (tmp_path_local / directory).mkdir() + transport.mkdir(str(tmp_path_remote / directory)) + + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' + + remote_file_name = 'file_remote.txt' + # here use full path in src and dst + local_file_abs_path = str(tmp_path_local / directory / local_file_name) + retrieved_file_abs_path = str(tmp_path_local / directory / retrieved_file_name) + remote_file_abs_path = str(tmp_path_remote / directory / remote_file_name) + + text = 'Viva Verdi\n' + with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: + fhandle.write(text) + + transport.put(local_file_abs_path, remote_file_abs_path) + transport.get(remote_file_abs_path, retrieved_file_abs_path) + + list_of_files = transport.listdir(str(tmp_path_remote / directory)) + # it is False because local_file_name has the full path, + # while list_of_files has not + assert local_file_name not in list_of_files + assert remote_file_name in list_of_files + assert retrieved_file_name not in list_of_files -def test_put_and_get_file(custom_transport): + +def test_putfile_and_getfile(custom_transport, tmp_path_remote, tmp_path_local): """Test putting and getting files.""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) - transport.mkdir(directory) - transport.chdir(directory) + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' - local_file_name = os.path.join(local_dir, directory, 'file.txt') remote_file_name = 'file_remote.txt' - retrieved_file_name = os.path.join(local_dir, directory, 'file_retrieved.txt') + + # here use full path in src and dst + local_file_abs_path = str(local_dir / directory / local_file_name) + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) - # here use full path in src and dst - transport.put(local_file_name, remote_file_name) - transport.get(remote_file_name, retrieved_file_name) - transport.putfile(local_file_name, remote_file_name) - transport.getfile(remote_file_name, retrieved_file_name) + transport.putfile(local_file_abs_path, remote_file_abs_path) + transport.getfile(remote_file_abs_path, retrieved_file_abs_path) - list_of_files = transport.listdir('.') + list_of_files = transport.listdir(str(remote_dir / directory)) # it is False because local_file_name has the full path, # while list_of_files has not assert local_file_name not in list_of_files assert remote_file_name in list_of_files assert retrieved_file_name not in list_of_files - os.remove(local_file_name) - transport.remove(remote_file_name) - os.remove(retrieved_file_name) - - transport.chdir('..') - transport.rmdir(directory) - -def test_put_get_abs_path_file(custom_transport): +def test_put_get_abs_path_file(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception for non existing files and abs path""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) - transport.mkdir(directory) - transport.chdir(directory) + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' - partial_file_name = 'file.txt' - local_file_name = os.path.join(local_dir, directory, 'file.txt') remote_file_name = 'file_remote.txt' - retrieved_file_name = os.path.join(local_dir, directory, 'file_retrieved.txt') + local_file_rel_path = local_file_name + remote_file_rel_path = remote_file_name - pathlib.Path(local_file_name).touch() + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) # partial_file_name is not an abs path with pytest.raises(ValueError): - transport.put(partial_file_name, remote_file_name) + transport.put(local_file_rel_path, remote_file_abs_path) with pytest.raises(ValueError): - transport.putfile(partial_file_name, remote_file_name) + transport.putfile(local_file_rel_path, remote_file_abs_path) # retrieved_file_name does not exist with pytest.raises(OSError): - transport.put(retrieved_file_name, remote_file_name) + transport.put(retrieved_file_abs_path, remote_file_abs_path) with pytest.raises(OSError): - transport.putfile(retrieved_file_name, remote_file_name) + transport.putfile(retrieved_file_abs_path, remote_file_abs_path) # remote_file_name does not exist with pytest.raises(OSError): - transport.get(remote_file_name, retrieved_file_name) + transport.get(remote_file_abs_path, retrieved_file_abs_path) with pytest.raises(OSError): - transport.getfile(remote_file_name, retrieved_file_name) - - transport.put(local_file_name, remote_file_name) - transport.putfile(local_file_name, remote_file_name) + transport.getfile(remote_file_abs_path, retrieved_file_abs_path) - # local filename is not an abs path + # remote filename is not an abs path with pytest.raises(ValueError): - transport.get(remote_file_name, 'delete_me.txt') + transport.get(remote_file_rel_path, 'delete_me.txt') with pytest.raises(ValueError): - transport.getfile(remote_file_name, 'delete_me.txt') + transport.getfile(remote_file_rel_path, 'delete_me.txt') - transport.remove(remote_file_name) - os.remove(local_file_name) - transport.chdir('..') - transport.rmdir(directory) - - -def test_put_get_empty_string_file(custom_transport): +def test_put_get_empty_string_file(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception put/get of empty strings""" - # TODO : verify the correctness of \n at the end of a file - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) - transport.mkdir(directory) - transport.chdir(directory) + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' - local_file_name = os.path.join(local_dir, directory, 'file_local.txt') remote_file_name = 'file_remote.txt' - retrieved_file_name = os.path.join(local_dir, directory, 'file_retrieved.txt') + + # here use full path in src and dst + local_file_abs_path = str(local_dir / directory / local_file_name) + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) # localpath is an empty string # ValueError because it is not an abs path with pytest.raises(ValueError): - transport.put('', remote_file_name) + transport.put('', remote_file_abs_path) with pytest.raises(ValueError): - transport.putfile('', remote_file_name) + transport.putfile('', remote_file_abs_path) # remote path is an empty string with pytest.raises(OSError): - transport.put(local_file_name, '') + transport.put(local_file_abs_path, '') with pytest.raises(OSError): - transport.putfile(local_file_name, '') + transport.putfile(local_file_abs_path, '') - transport.put(local_file_name, remote_file_name) + transport.put(local_file_abs_path, remote_file_abs_path) # overwrite the remote_file_name - transport.putfile(local_file_name, remote_file_name) + transport.putfile(local_file_abs_path, remote_file_abs_path) # remote path is an empty string with pytest.raises(OSError): - transport.get('', retrieved_file_name) + transport.get('', retrieved_file_abs_path) with pytest.raises(OSError): - transport.getfile('', retrieved_file_name) + transport.getfile('', retrieved_file_abs_path) # local path is an empty string # ValueError because it is not an abs path with pytest.raises(ValueError): - transport.get(remote_file_name, '') + transport.get(remote_file_abs_path, '') with pytest.raises(ValueError): - transport.getfile(remote_file_name, '') + transport.getfile(remote_file_abs_path, '') # TODO : get doesn't retrieve empty files. # Is it what we want? - transport.get(remote_file_name, retrieved_file_name) - # overwrite retrieved_file_name - transport.getfile(remote_file_name, retrieved_file_name) + transport.get(remote_file_abs_path, retrieved_file_abs_path) + assert Path(retrieved_file_abs_path).exists() + t1 = Path(retrieved_file_abs_path).stat().st_mtime_ns - os.remove(local_file_name) - transport.remove(remote_file_name) - # If it couldn't end the copy, it leaves what he did on - # local file - assert 'file_retrieved.txt' in transport.listdir('.') - os.remove(retrieved_file_name) + # overwrite retrieved_file_name in 0.01 s + time.sleep(0.01) + transport.getfile(remote_file_abs_path, retrieved_file_abs_path) + assert Path(retrieved_file_abs_path).exists() + t2 = Path(retrieved_file_abs_path).stat().st_mtime_ns - transport.chdir('..') - transport.rmdir(directory) + # Check st_mtime_ns to sure it is override + assert t2 > t1 -def test_put_and_get_tree(custom_transport): +def test_put_and_get_tree(custom_transport, tmp_path_remote, tmp_path_local): """Test putting and getting files.""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - - local_subfolder = os.path.join(local_dir, directory, 'tmp1') - remote_subfolder = 'tmp2' - retrieved_subfolder = os.path.join(local_dir, directory, 'tmp3') + local_subfolder: Path = local_dir / directory / 'tmp1' + remote_subfolder: Path = remote_dir / 'tmp2' + retrieved_subfolder: Path = local_dir / directory / 'tmp3' - os.mkdir(os.path.join(local_dir, directory)) - os.mkdir(os.path.join(local_dir, directory, local_subfolder)) + local_subfolder.mkdir(parents=True) - transport.chdir(directory) - - local_file_name = os.path.join(local_subfolder, 'file.txt') + local_file = local_subfolder / 'file.txt' text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file, 'w', encoding='utf8') as fhandle: fhandle.write(text) # here use full path in src and dst - for i in range(2): - if i == 0: - transport.put(local_subfolder, remote_subfolder) - transport.get(remote_subfolder, retrieved_subfolder) - else: - transport.puttree(local_subfolder, remote_subfolder) - transport.gettree(remote_subfolder, retrieved_subfolder) - - # Here I am mixing the local with the remote fold - list_of_dirs = transport.listdir('.') - # # it is False because local_file_name has the full path, - # # while list_of_files has not - assert local_subfolder not in list_of_dirs - assert remote_subfolder in list_of_dirs - assert retrieved_subfolder not in list_of_dirs - assert 'tmp1' in list_of_dirs - assert 'tmp3' in list_of_dirs - - list_pushed_file = transport.listdir('tmp2') - list_retrieved_file = transport.listdir('tmp3') - assert 'file.txt' in list_pushed_file - assert 'file.txt' in list_retrieved_file - - shutil.rmtree(local_subfolder) - shutil.rmtree(retrieved_subfolder) - transport.rmtree(remote_subfolder) - - transport.chdir('..') - transport.rmdir(directory) + transport.puttree(str(local_subfolder), str(remote_subfolder)) + transport.gettree(str(remote_subfolder), str(retrieved_subfolder)) + + list_of_dirs = [p.name for p in (local_dir / directory).iterdir()] + + assert local_subfolder not in list_of_dirs + assert remote_subfolder not in list_of_dirs + assert retrieved_subfolder not in list_of_dirs + assert 'tmp1' in list_of_dirs + assert 'tmp3' in list_of_dirs + + list_pushed_file = transport.listdir(str(remote_subfolder)) + list_retrieved_file = [p.name for p in retrieved_subfolder.iterdir()] + assert 'file.txt' in list_pushed_file + assert 'file.txt' in list_retrieved_file @pytest.mark.parametrize( @@ -711,251 +620,245 @@ def test_put_and_get_overwrite( ) -def test_copy(custom_transport): - """Test copying.""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir +def test_copy(custom_transport, tmp_path_remote): + """Test copying from a remote src to remote dst""" + remote_dir = tmp_path_remote + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) + workdir = remote_dir / directory - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + transport.mkdir(str(workdir)) - transport.mkdir(directory) - transport.chdir(directory) - - local_base_dir = os.path.join(local_dir, directory, 'local') - os.mkdir(local_base_dir) + base_dir = workdir / 'origin' + base_dir.mkdir() - # first test put: I create three files in local - file_1 = os.path.join(local_base_dir, 'a.txt') - file_2 = os.path.join(local_base_dir, 'b.tmp') - file_3 = os.path.join(local_base_dir, 'c.txt') + # first create three files + file_1 = base_dir / 'a.txt' + file_2 = base_dir / 'b.tmp' + file_3 = base_dir / 'c.txt' text = 'Viva Verdi\n' for filename in [file_1, file_2, file_3]: with open(filename, 'w', encoding='utf8') as fhandle: fhandle.write(text) # first test the copy. Copy of two files matching patterns, into a folder - transport.copy(os.path.join('local', '*.txt'), '.') - assert set(['a.txt', 'c.txt', 'local']) == set(transport.listdir('.')) - transport.remove('a.txt') - transport.remove('c.txt') + transport.copy(str(base_dir / '*.txt'), str(workdir)) + assert set(['a.txt', 'c.txt', 'origin']) == set(transport.listdir(str(workdir))) + transport.remove(str(workdir / 'a.txt')) + transport.remove(str(workdir / 'c.txt')) + # second test copy. Copy of two folders - transport.copy('local', 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir('prova')) - transport.rmtree('prova') + transport.copy(str(base_dir), str(workdir / 'prova')) + assert set(['prova', 'origin']) == set(transport.listdir(str(workdir))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(workdir / 'prova'))) + transport.rmtree(str(workdir / 'prova')) + # third test copy. Can copy one file into a new file - transport.copy(os.path.join('local', '*.tmp'), 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - transport.remove('prova') + transport.copy(str(base_dir / '*.tmp'), str(workdir / 'prova')) + assert transport.isfile(str(workdir / 'prova')) + transport.remove(str(workdir / 'prova')) + # fourth test copy: can't copy more than one file on the same file, # i.e., the destination should be a folder with pytest.raises(OSError): - transport.copy(os.path.join('local', '*.txt'), 'prova') + transport.copy(str(base_dir / '*.txt'), str(workdir / 'prova')) + # fifth test, copying one file into a folder - transport.mkdir('prova') - transport.copy(os.path.join('local', 'a.txt'), 'prova') - assert set(transport.listdir('prova')) == set(['a.txt']) - transport.rmtree('prova') + transport.mkdir(str(workdir / 'prova')) + transport.copy(str(base_dir / 'a.txt'), str(workdir / 'prova')) + assert set(transport.listdir(str(workdir / 'prova'))) == set(['a.txt']) + transport.rmtree(str(workdir / 'prova')) + # sixth test, copying one file into a file - transport.copy(os.path.join('local', 'a.txt'), 'prova') - assert transport.isfile('prova') - transport.remove('prova') + transport.copy(str(base_dir / 'a.txt'), str(workdir / 'prova')) + assert transport.isfile(str(workdir / 'prova')) + transport.remove(str(workdir / 'prova')) # copy of folder into an existing folder # NOTE: the command cp has a different behavior on Mac vs Ubuntu # tests performed locally on a Mac may result in a failure. - transport.mkdir('prova') - transport.copy('local', 'prova') - assert set(['local']) == set(transport.listdir('prova')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(os.path.join('prova', 'local'))) - transport.rmtree('prova') + transport.mkdir(str(workdir / 'prova')) + transport.copy(str(base_dir), str(workdir / 'prova')) + assert set(['origin']) == set(transport.listdir(str(workdir / 'prova'))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(workdir / 'prova' / 'origin'))) + transport.rmtree(str(workdir / 'prova')) # exit - transport.chdir('..') - transport.rmtree(directory) + transport.rmtree(str(workdir)) -def test_put(custom_transport): - """Test putting files.""" - # exactly the same tests of copy, just with the put function - # and therefore the local path must be absolute - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir +def test_put(custom_transport, tmp_path_remote, tmp_path_local): + """Test putting files. + Those are similar tests of copy, just with the put function which copy from mocked local to mocked remote + and therefore the local path must be absolute + """ + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) + local_workdir = local_dir / directory + remote_workdir = remote_dir / directory - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + transport.mkdir(str(remote_workdir)) - transport.mkdir(directory) - transport.chdir(directory) - - local_base_dir = os.path.join(local_dir, directory, 'local') - os.mkdir(local_base_dir) + local_base_dir: Path = local_workdir / 'origin' + local_base_dir.mkdir(parents=True) # first test put: I create three files in local - file_1 = os.path.join(local_base_dir, 'a.txt') - file_2 = os.path.join(local_base_dir, 'b.tmp') - file_3 = os.path.join(local_base_dir, 'c.txt') + file_1 = local_base_dir / 'a.txt' + file_2 = local_base_dir / 'b.tmp' + file_3 = local_base_dir / 'c.txt' text = 'Viva Verdi\n' for filename in [file_1, file_2, file_3]: with open(filename, 'w', encoding='utf8') as fhandle: fhandle.write(text) - # first test putransport. Copy of two files matching patterns, into a folder - transport.put(os.path.join(local_base_dir, '*.txt'), '.') - assert set(['a.txt', 'c.txt', 'local']) == set(transport.listdir('.')) - transport.remove('a.txt') - transport.remove('c.txt') - # second. Copy of folder into a non existing folder - transport.put(local_base_dir, 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir('prova')) - transport.rmtree('prova') - # third. copy of folder into an existing folder - transport.mkdir('prova') - transport.put(local_base_dir, 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - assert set(['local']) == set(transport.listdir('prova')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(os.path.join('prova', 'local'))) - transport.rmtree('prova') - # third test copy. Can copy one file into a new file - transport.put(os.path.join(local_base_dir, '*.tmp'), 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - transport.remove('prova') - # fourth test copy: can't copy more than one file on the same file, + # first test the put. Copy of two files matching patterns, into a folder + transport.put(str(local_base_dir / '*.txt'), str(remote_workdir)) + assert set(['a.txt', 'c.txt']) == set(transport.listdir(str(remote_workdir))) + transport.remove(str(remote_workdir / 'a.txt')) + transport.remove(str(remote_workdir / 'c.txt')) + + # second test put. Put of two folders + transport.put(str(local_base_dir), str(remote_workdir / 'prova')) + assert set(['prova']) == set(transport.listdir(str(remote_workdir))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(remote_workdir / 'prova'))) + transport.rmtree(str(remote_workdir / 'prova')) + + # third test put. Can copy one file into a new file + transport.put(str(local_base_dir / '*.tmp'), str(remote_workdir / 'prova')) + assert transport.isfile(str(remote_workdir / 'prova')) + transport.remove(str(remote_workdir / 'prova')) + + # fourth test put: can't copy more than one file to the same file, # i.e., the destination should be a folder with pytest.raises(OSError): - transport.put(os.path.join(local_base_dir, '*.txt'), 'prova') - # copy of folder into file - with open(os.path.join(local_dir, directory, 'existing.txt'), 'w', encoding='utf8') as fhandle: + transport.put(str(local_base_dir / '*.txt'), str(remote_workdir / 'prova')) + + # can't copy folder to an exist file + with open(remote_workdir / 'existing.txt', 'w', encoding='utf8') as fhandle: fhandle.write(text) with pytest.raises(OSError): - transport.put(os.path.join(local_base_dir), 'existing.txt') - transport.remove('existing.txt') + transport.put(str(local_base_dir), str(remote_workdir / 'existing.txt')) + transport.remove(str(remote_workdir / 'existing.txt')) + # fifth test, copying one file into a folder - transport.mkdir('prova') - transport.put(os.path.join(local_base_dir, 'a.txt'), 'prova') - assert set(transport.listdir('prova')) == set(['a.txt']) - transport.rmtree('prova') + transport.mkdir(str(remote_workdir / 'prova')) + transport.put(str(local_base_dir / 'a.txt'), str(remote_workdir / 'prova')) + assert set(transport.listdir(str(remote_workdir / 'prova'))) == set(['a.txt']) + transport.rmtree(str(remote_workdir / 'prova')) + # sixth test, copying one file into a file - transport.put(os.path.join(local_base_dir, 'a.txt'), 'prova') - assert transport.isfile('prova') - transport.remove('prova') + transport.put(str(local_base_dir / 'a.txt'), str(remote_workdir / 'prova')) + assert transport.isfile(str(remote_workdir / 'prova')) + transport.remove(str(remote_workdir / 'prova')) + # put of folder into an existing folder + # NOTE: the command cp has a different behavior on Mac vs Ubuntu + # tests performed locally on a Mac may result in a failure. + transport.mkdir(str(remote_workdir / 'prova')) + transport.put(str(local_base_dir), str(remote_workdir / 'prova')) + assert set(['origin']) == set(transport.listdir(str(remote_workdir / 'prova'))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(remote_workdir / 'prova' / 'origin'))) + transport.rmtree(str(remote_workdir / 'prova')) # exit - transport.chdir('..') - transport.rmtree(directory) + transport.rmtree(str(remote_workdir)) -def test_get(custom_transport): +def test_get(custom_transport, tmp_path_remote, tmp_path_local): """Test getting files.""" - # exactly the same tests of copy, just with the put function - # and therefore the local path must be absolute - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + local_workdir: Path = local_dir / directory + remote_workdir: Path = remote_dir / directory - transport.mkdir(directory) - transport.chdir(directory) + local_workdir.mkdir() - local_base_dir = os.path.join(local_dir, directory, 'local') - local_destination = os.path.join(local_dir, directory) - os.mkdir(local_base_dir) + remote_base_dir: Path = remote_workdir / 'origin' + remote_base_dir.mkdir(parents=True) - # first test put: I create three files in local - file_1 = os.path.join(local_base_dir, 'a.txt') - file_2 = os.path.join(local_base_dir, 'b.tmp') - file_3 = os.path.join(local_base_dir, 'c.txt') + # first test put: I create three files in remote + file_1 = remote_base_dir / 'a.txt' + file_2 = remote_base_dir / 'b.tmp' + file_3 = remote_base_dir / 'c.txt' text = 'Viva Verdi\n' for filename in [file_1, file_2, file_3]: with open(filename, 'w', encoding='utf8') as fhandle: fhandle.write(text) - # first test put. Copy of two files matching patterns, into a folder - transport.get(os.path.join('local', '*.txt'), local_destination) - assert set(['a.txt', 'c.txt', 'local']) == set(os.listdir(local_destination)) - os.remove(os.path.join(local_destination, 'a.txt')) - os.remove(os.path.join(local_destination, 'c.txt')) + # first test get. Get two files matching patterns, from mocked remote folder into a local folder + transport.get(str(remote_base_dir / '*.txt'), str(local_workdir)) + assert set(['a.txt', 'c.txt']) == set([p.name for p in (local_workdir).iterdir()]) + (local_workdir / 'a.txt').unlink() + (local_workdir / 'c.txt').unlink() + # second. Copy of folder into a non existing folder - transport.get('local', os.path.join(local_destination, 'prova')) - assert set(['prova', 'local']) == set(os.listdir(local_destination)) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(os.listdir(os.path.join(local_destination, 'prova'))) - shutil.rmtree(os.path.join(local_destination, 'prova')) + transport.get(str(remote_base_dir), str(local_workdir / 'prova')) + assert set(['prova']) == set([p.name for p in local_workdir.iterdir()]) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set([p.name for p in (local_workdir / 'prova').iterdir()]) + shutil.rmtree(local_workdir / 'prova') + # third. copy of folder into an existing folder - os.mkdir(os.path.join(local_destination, 'prova')) - transport.get('local', os.path.join(local_destination, 'prova')) - assert set(['prova', 'local']) == set(os.listdir(local_destination)) - assert set(['local']) == set(os.listdir(os.path.join(local_destination, 'prova'))) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(os.listdir(os.path.join(local_destination, 'prova', 'local'))) - shutil.rmtree(os.path.join(local_destination, 'prova')) - # third test copy. Can copy one file into a new file - transport.get(os.path.join('local', '*.tmp'), os.path.join(local_destination, 'prova')) - assert set(['prova', 'local']) == set(os.listdir(local_destination)) - os.remove(os.path.join(local_destination, 'prova')) + (local_workdir / 'prova').mkdir() + transport.get(str(remote_base_dir), str(local_workdir / 'prova')) + assert set(['prova']) == set([p.name for p in local_workdir.iterdir()]) + assert set(['origin']) == set([p.name for p in (local_workdir / 'prova').iterdir()]) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set([p.name for p in (local_workdir / 'prova' / 'origin').iterdir()]) + shutil.rmtree(local_workdir / 'prova') + + # test get one file into a new file prova + transport.get(str(remote_base_dir / '*.tmp'), str(local_workdir / 'prova')) + assert set(['prova']) == set([p.name for p in local_workdir.iterdir()]) + assert (local_workdir / 'prova').is_file() + (local_workdir / 'prova').unlink() + # fourth test copy: can't copy more than one file on the same file, # i.e., the destination should be a folder with pytest.raises(OSError): - transport.get(os.path.join('local', '*.txt'), os.path.join(local_destination, 'prova')) + transport.get(str(remote_base_dir / '*.txt'), str(local_workdir / 'prova')) # copy of folder into file - with open(os.path.join(local_destination, 'existing.txt'), 'w', encoding='utf8') as fhandle: + with open(local_workdir / 'existing.txt', 'w', encoding='utf8') as fhandle: fhandle.write(text) with pytest.raises(OSError): - transport.get('local', os.path.join(local_destination, 'existing.txt')) - os.remove(os.path.join(local_destination, 'existing.txt')) + transport.get(str(remote_base_dir), str(local_workdir / 'existing.txt')) + (local_workdir / 'existing.txt').unlink() + # fifth test, copying one file into a folder - os.mkdir(os.path.join(local_destination, 'prova')) - transport.get(os.path.join('local', 'a.txt'), os.path.join(local_destination, 'prova')) - assert set(os.listdir(os.path.join(local_destination, 'prova'))) == set(['a.txt']) - shutil.rmtree(os.path.join(local_destination, 'prova')) - # sixth test, copying one file into a file - transport.get(os.path.join('local', 'a.txt'), os.path.join(local_destination, 'prova')) - assert os.path.isfile(os.path.join(local_destination, 'prova')) - os.remove(os.path.join(local_destination, 'prova')) + (local_workdir / 'prova').mkdir() + transport.get(str(remote_base_dir / 'a.txt'), str(local_workdir / 'prova')) + assert set(['a.txt']) == set([p.name for p in (local_workdir / 'prova').iterdir()]) + shutil.rmtree(local_workdir / 'prova') - # exit - transport.chdir('..') - transport.rmtree(directory) + # sixth test, copying one file into a file + transport.get(str(remote_base_dir / 'a.txt'), str(local_workdir / 'prova')) + assert (local_workdir / 'prova').is_file() + (local_workdir / 'prova').unlink() -def test_put_get_abs_path_tree(custom_transport): +def test_put_get_abs_path_tree(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception for non existing files and abs path""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) + local_subfolder = str(local_dir / directory / 'tmp1') + remote_subfolder = str(remote_dir / 'tmp2') + retrieved_subfolder = str(local_dir / directory / 'tmp3') - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + (local_dir / directory / local_subfolder).mkdir(parents=True) - local_subfolder = os.path.join(local_dir, directory, 'tmp1') - remote_subfolder = 'tmp2' - retrieved_subfolder = os.path.join(local_dir, directory, 'tmp3') + local_file_name = Path(local_subfolder) / 'file.txt' - os.mkdir(os.path.join(local_dir, directory)) - os.mkdir(os.path.join(local_dir, directory, local_subfolder)) - - transport.chdir(directory) - local_file_name = os.path.join(local_subfolder, 'file.txt') - pathlib.Path(local_file_name).touch() + text = 'Viva Verdi\n' + with open(local_file_name, 'w', encoding='utf8') as fhandle: + fhandle.write(text) + # here use full path in src and dst # 'tmp1' is not an abs path with pytest.raises(ValueError): transport.put('tmp1', remote_subfolder) @@ -990,40 +893,24 @@ def test_put_get_abs_path_tree(custom_transport): with pytest.raises(ValueError): transport.gettree(remote_subfolder, 'delete_me_tree') - os.remove(os.path.join(local_subfolder, 'file.txt')) - os.rmdir(local_subfolder) - transport.rmtree(remote_subfolder) - transport.chdir('..') - transport.rmdir(directory) - - -def test_put_get_empty_string_tree(custom_transport): +def test_put_get_empty_string_tree(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception put/get of empty strings""" - # TODO : verify the correctness of \n at the end of a file - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + local_subfolder: Path = local_dir / directory / 'tmp1' + remote_subfolder: Path = remote_dir / 'tmp2' + retrieved_subfolder: Path = local_dir / directory / 'tmp3' - local_subfolder = os.path.join(local_dir, directory, 'tmp1') - remote_subfolder = 'tmp2' - retrieved_subfolder = os.path.join(local_dir, directory, 'tmp3') + local_subfolder.mkdir(parents=True) - os.mkdir(os.path.join(local_dir, directory)) - os.mkdir(os.path.join(local_dir, directory, local_subfolder)) - - transport.chdir(directory) - local_file_name = os.path.join(local_subfolder, 'file.txt') + local_file = local_subfolder / 'file.txt' text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file, 'w', encoding='utf8') as fhandle: fhandle.write(text) # localpath is an empty string @@ -1035,7 +922,7 @@ def test_put_get_empty_string_tree(custom_transport): with pytest.raises(OSError): transport.puttree(local_subfolder, '') - transport.puttree(local_subfolder, remote_subfolder) + transport.puttree(str(local_subfolder), str(remote_subfolder)) # remote path is an empty string with pytest.raises(OSError): @@ -1048,73 +935,50 @@ def test_put_get_empty_string_tree(custom_transport): # TODO : get doesn't retrieve empty files. # Is it what we want? - transport.gettree(remote_subfolder, retrieved_subfolder) - - os.remove(os.path.join(local_subfolder, 'file.txt')) - os.rmdir(local_subfolder) - transport.remove(os.path.join(remote_subfolder, 'file.txt')) - transport.rmdir(remote_subfolder) - # If it couldn't end the copy, it leaves what he did on local file - # here I am mixing local with remote - assert 'file.txt' in transport.listdir('tmp3') - os.remove(os.path.join(retrieved_subfolder, 'file.txt')) - os.rmdir(retrieved_subfolder) + transport.gettree(str(remote_subfolder), str(retrieved_subfolder)) - transport.chdir('..') - transport.rmdir(directory) + assert 'file.txt' in [p.name for p in retrieved_subfolder.iterdir()] -def test_gettree_nested_directory(custom_transport): +def test_gettree_nested_directory(custom_transport, tmp_path_remote, tmp_path_local): """Test `gettree` for a nested directory.""" - with tempfile.TemporaryDirectory() as dir_remote, tempfile.TemporaryDirectory() as dir_local: - content = b'dummy\ncontent' - filepath = os.path.join(dir_remote, 'sub', 'path', 'filename.txt') - os.makedirs(os.path.dirname(filepath)) + content = b'dummy\ncontent' + dir_path = tmp_path_remote / 'sub' / 'path' + dir_path.mkdir(parents=True) + + file_path = str(dir_path / 'filename.txt') + + with open(file_path, 'wb') as handle: + handle.write(content) - with open(filepath, 'wb') as handle: - handle.write(content) + with custom_transport as transport: + transport.gettree(str(tmp_path_remote), str(tmp_path_local)) - with custom_transport as transport: - transport.gettree(os.path.join(dir_remote, 'sub/path'), os.path.join(dir_local, 'sub/path')) + assert (tmp_path_local / 'sub' / 'path' / 'filename.txt').is_file -def test_exec_pwd(custom_transport): +def test_exec_pwd(custom_transport, tmp_path_remote): """I create a strange subfolder with a complicated name and - then see if I can run pwd. This also checks the correct + then see if I can run ``ls``. This also checks the correct escaping of funny characters, both in the directory creation (which should be done by paramiko) and in the command execution (done in this module, in the _exec_command_internal function). """ # Start value - delete_at_end = False - with custom_transport as transport: # To compare with: getcwd uses the normalized ('realpath') path - location = transport.normalize('/tmp') subfolder = """_'s f"#""" # A folder with characters to escape - subfolder_fullpath = os.path.join(location, subfolder) + subfolder_fullpath = str(tmp_path_remote / subfolder) - transport.chdir(location) - if not transport.isdir(subfolder): - # Since I created the folder, I will remember to - # delete it at the end of this test - delete_at_end = True - transport.mkdir(subfolder) + transport.mkdir(subfolder_fullpath) - assert transport.isdir(subfolder) - transport.chdir(subfolder) + assert transport.isdir(subfolder_fullpath) - assert subfolder_fullpath == transport.getcwd() - retcode, stdout, stderr = transport.exec_command_wait('pwd') + retcode, stdout, stderr = transport.exec_command_wait(f'ls {tmp_path_remote!s}') assert retcode == 0 - # I have to strip it because 'pwd' returns a trailing \n - assert stdout.strip() == subfolder_fullpath + assert stdout.strip() in subfolder_fullpath assert stderr == '' - if delete_at_end: - transport.chdir(location) - transport.rmdir(subfolder) - def test_exec_with_stdin_string(custom_transport): """Test command execution with a stdin string.""" @@ -1195,7 +1059,7 @@ def test_exec_with_wrong_stdin(custom_transport): transport.exec_command_wait('cat', stdin=1) -def test_transfer_big_stdout(custom_transport): +def test_transfer_big_stdout(custom_transport, tmp_path_remote): """Test the transfer of a large amount of data on stdout.""" # Create a "big" file of > 2MB (10MB here; in general, larger than the buffer size) min_file_size_bytes = 5 * 1024 * 1024 @@ -1210,25 +1074,18 @@ def test_transfer_big_stdout(custom_transport): line_repetitions = min_file_size_bytes // len(file_line_binary) + 1 fcontent = (file_line_binary * line_repetitions).decode('utf8') - with custom_transport as trans: + with custom_transport as transport: # We cannot use tempfile.mkdtemp because we're on a remote folder - location = trans.normalize(os.path.join('/', 'tmp')) - trans.chdir(location) - assert location == trans.getcwd() - - directory = 'temp_dir_test_transfer_big_stdout' - while trans.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - trans.mkdir(directory) - trans.chdir(directory) + directory_name = 'temp_dir_test_transfer_big_stdout' + directory_path = tmp_path_remote / directory_name + transport.mkdir(str(directory_path)) with tempfile.NamedTemporaryFile(mode='wb') as tmpf: tmpf.write(fcontent.encode('utf8')) tmpf.flush() # I put a file with specific content there at the right file name - trans.putfile(tmpf.name, fname) + transport.putfile(tmpf.name, str(directory_path / fname)) python_code = r"""import sys @@ -1252,16 +1109,20 @@ def test_transfer_big_stdout(custom_transport): tmpf.flush() # I put a file with specific content there at the right file name - trans.putfile(tmpf.name, script_fname) + transport.putfile(tmpf.name, str(directory_path / script_fname)) # I get its content via the stdout; emulate also network slowness (note I cat twice) - retcode, stdout, stderr = trans.exec_command_wait(f'cat {fname} ; sleep 1 ; cat {fname}') + retcode, stdout, stderr = transport.exec_command_wait( + f'cat {fname} ; sleep 1 ; cat {fname}', workdir=str(directory_path) + ) assert stderr == '' assert stdout == fcontent + fcontent assert retcode == 0 # I get its content via the stderr; emulate also network slowness (note I cat twice) - retcode, stdout, stderr = trans.exec_command_wait(f'cat {fname} >&2 ; sleep 1 ; cat {fname} >&2') + retcode, stdout, stderr = transport.exec_command_wait( + f'cat {fname} >&2 ; sleep 1 ; cat {fname} >&2', workdir=str(directory_path) + ) assert stderr == fcontent + fcontent assert stdout == '' assert retcode == 0 @@ -1274,20 +1135,14 @@ def test_transfer_big_stdout(custom_transport): # line_repetitions, file_line, file_line)) # However this is pretty slow (and using 'cat' of a file containing only one line is even slower) - retcode, stdout, stderr = trans.exec_command_wait(f'python3 {script_fname}') + retcode, stdout, stderr = transport.exec_command_wait(f'python3 {script_fname}', workdir=str(directory_path)) assert stderr == fcontent assert stdout == fcontent assert retcode == 0 - # Clean-up - trans.remove(fname) - trans.remove(script_fname) - trans.chdir('..') - trans.rmdir(directory) - -def test_asynchronous_execution(custom_transport): +def test_asynchronous_execution(custom_transport, tmp_path): """Test that the execution of a long(ish) command via the direct scheduler does not block. This is a regression test for #3094, where running a long job on the direct scheduler @@ -1295,6 +1150,8 @@ def test_asynchronous_execution(custom_transport): """ # Use a unique name, using a UUID, to avoid concurrent tests (or very rapid # tests that follow each other) to overwrite the same destination + import os + script_fname = f'sleep-submit-{uuid.uuid4().hex}-{custom_transport.__class__.__name__}.sh' scheduler = SchedulerFactory('core.direct')() @@ -1306,10 +1163,10 @@ def test_asynchronous_execution(custom_transport): tmpf.write(b'#!/bin/bash\nsleep 10\n') tmpf.flush() - transport.putfile(tmpf.name, os.path.join('/tmp', script_fname)) + transport.putfile(tmpf.name, str(tmp_path / script_fname)) timestamp_before = time.time() - job_id_string = scheduler.submit_job('/tmp', script_fname) + job_id_string = scheduler.submit_job(str(tmp_path), script_fname) elapsed_time = time.time() - timestamp_before # We want to get back control. If it takes < 5 seconds, it means that it is not blocking @@ -1344,10 +1201,3 @@ def test_asynchronous_execution(custom_transport): except ProcessLookupError: # If the process is already dead (or has never run), I just ignore the error pass - - # Also remove the script - try: - transport.remove(f'/tmp/{script_fname}') - except FileNotFoundError: - # If the file wasn't even created, I just ignore this error - pass