DataLab-Platform / DataLab

Open-source Platform for Scientific and Technical Data Processing and Visualization
https://datalab-platform.com
BSD 3-Clause "New" or "Revised" License
43 stars 4 forks source link

Test data paths may be added multiple times to `cdl.utils.tests.TST_PATH` #85

Closed PierreRaybaut closed 3 months ago

PierreRaybaut commented 3 months ago

While writing the post-release V0.16.2 report regarding issues with Debian packaging (see #84), I realized that test data paths may be added multiple times to the global list of strings cdl.utils.tests.TST_PATH. Test utility functions such as get_test_fnames rely on this list of paths to collect data files for testing purpose.

However, path were added to this list without checking anything - and especially without checking if the added path had already been added to the list. This was the real cause of the reorder_app_test failure reported in #84: the "reorder.h5" data file was loaded twice in DataLab workspace because the data path (cdl/tests/data*) was added twice to the TST_PATH list.

This has to be fixed by adding the following lines to cdl.utils.tests:

def add_test_path(path: str) -> None:
    """Appends test data path, after normalizing it and making it absolute.
    Do nothing if the path is already in the list.

    Args:
        Path to add to the list of test data paths

    Raises:
        FileNotFoundError: if the path does not exist
    """
    path = osp.abspath(osp.normpath(path))
    if path not in TST_PATH:
        if not osp.exists(path):
            raise FileNotFoundError(f"Test data path does not exist: {path}")
        TST_PATH.append(path)

def add_test_path_from_env(envvar: str) -> None:
    """Appends test data path from environment variable (fails silently)"""
    # Note: this function is used in third-party plugins
    path = os.environ.get(envvar)
    if path:
        add_test_path(path)

# Add test data files and folders pointed by `CDL_DATA` environment variable:
add_test_path_from_env("CDL_DATA")
PierreRaybaut commented 3 months ago

Fix with d11b9239635ff85ca90c1a66ddb4ac8100a411f6