Refactor some test fixtures for better reuse so we can test loading Scheduler config examples

[bpkroth]

Pull Request

Title

Refactor some test fixtures for better reuse so we can test loading Scheduler config examples

---

Description

• Moves temporary file based sqlite Storage fixture to a reusable location.
• Move Optimizer fixtures to reusable location following Storage fixtures pattern.
• Uses both in setting up TrialRunner fixtures for testing Scheduler config examples.
• Also be sure to load all test configs too.

---

Type of Change

• 🔄 Refactor
• 🧪 Tests

---

Testing

• Introduces new CI test for Scheduler configs that follows the usual pattern for other objects (Storage, Optimizer, Environments, Services, etc.)

---

Additional Notes (optional)

• Split out from Introduce mock_trial_data for MockEnv and add some basic Scheduler tests #980, WIP: Prepare Experiment.load to handle async out of order trial completion #973

---

[Copilot]

Pull Request Overview

Refactors test fixtures to improve reuse and introduces tests for loading Scheduler configuration examples.

• Centralizes the temporary file–based SQLite storage fixture (sqlite_storage) for reuse.
• Consolidates optimizer fixtures into a single module and re-exports them in conftest.py.
• Adds end-to-end tests to verify loading of Scheduler config examples with the new fixtures.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
mlos_bench/mlos_bench/tests/storage/test_storage_pickling.py	Updated to use the new sqlite_storage fixture; removed inline tempfile logic.
mlos_bench/mlos_bench/tests/storage/sql/fixtures.py	Added sqlite_storage fixture for file-based SQLite storage.
mlos_bench/mlos_bench/tests/storage/conftest.py	Exposed sqlite_storage fixture for pytest.
mlos_bench/mlos_bench/tests/optimizers/fixtures.py	New module centralizing all optimizer fixtures.
mlos_bench/mlos_bench/tests/optimizers/conftest.py	Re-exported optimizer fixtures from the new module.
mlos_bench/mlos_bench/tests/config/schedulers/test_load_scheduler_config_examples.py	Introduced tests for loading Scheduler config examples.
mlos_bench/mlos_bench/tests/config/schedulers/conftest.py	Added fixtures for mock environment config path and trial runners.
mlos_bench/mlos_bench/tests/config/schedulers/init.py	Added package docstring for scheduler config tests.

Comments suppressed due to low confidence (1)

mlos_bench/mlos_bench/tests/storage/test_storage_pickling.py:22

• pytest is not imported in this file, causing a NameError for @pytest.mark.skipif. Add import pytest at the top.

@pytest.mark.skipif(

[motus]

That's an interesting error. It looks like in one of the unit tests (test_load_scheduler_config_examples.py::test_load_scheduler_config_examples) we are removing a temp directory with SQLite DB file in it - while having another (?) thread/process with an open connection to that DB. Here's the error log:

_ ERROR at teardown of test_load_scheduler_config_examples[D:/a/MLOS/MLOS/mlos_bench/mlos_bench/config/schedulers/sync_scheduler.jsonc] _
[gw1] win32 -- Python 3.12.10 C:\Miniconda\envs\mlos\python.exe

path = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
onexc = <function TemporaryDirectory._rmtree.<locals>.onexc at 0x000001E9E466A2A0>

    def _rmtree_unsafe(path, onexc):
        def onerror(err):
            onexc(os.scandir, err.filename, err)
        results = os.walk(path, topdown=False, onerror=onerror, followlinks=os._walk_symlinks_as_files)
        for dirpath, dirnames, filenames in results:
            for name in dirnames:
                fullname = os.path.join(dirpath, name)
                try:
                    os.rmdir(fullname)
                except OSError as err:
                    onexc(os.rmdir, fullname, err)
            for name in filenames:
                fullname = os.path.join(dirpath, name)
                try:
>                   os.unlink(fullname)
E                   PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'

dirnames   = []
dirpath    = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
filenames  = ['mlos_bench.sqlite']
fullname   = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'
name       = 'mlos_bench.sqlite'
onerror    = <function _rmtree_unsafe.<locals>.onerror at 0x000001E9E466A660>
onexc      = <function TemporaryDirectory._rmtree.<locals>.onexc at 0x000001E9E466A2A0>
path       = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
results    = <generator object walk at 0x000001E9E2087CA0>

C:\Miniconda\envs\mlos\Lib\shutil.py:633: PermissionError

During handling of the above exception, another exception occurred:

    @pytest.fixture(scope="function")
    def sqlite_storage() -> Generator[SqlStorage]:
        """
        Fixture for file based SQLite storage in a temporary directory.
    
        Yields
        ------
        Generator[SqlStorage]
    
        Notes
        -----
        Can't be used in parallel tests on Windows.
        """
>       with tempfile.TemporaryDirectory() as tmpdir:

config_str = ('{"class": "mlos_bench.storage.sql.storage.SqlStorage", "config": '
 '{"drivername": "sqlite", "database": '
 '"C:\\\\Users\\\\RUNNER~1\\\\AppData\\\\Local\\\\Temp\\\\tmpuwby3eio\\\\mlos_bench.sqlite", '
 '"lazy_schema_create": false}}')
db_path    = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'
storage    = sqlite:C:\Users\RUNNER~1\AppData\Local\Temp\tmpuwby3eio\mlos_bench.sqlite
tmpdir     = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'

mlos_bench\mlos_bench\tests\storage\sql\fixtures.py:46: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
C:\Miniconda\envs\mlos\Lib\tempfile.py:950: in __exit__
    self.cleanup()
        exc        = None
        self       = <TemporaryDirectory 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'>
        tb         = None
        value      = None
C:\Miniconda\envs\mlos\Lib\tempfile.py:954: in cleanup
    self._rmtree(self.name, ignore_errors=self._ignore_cleanup_errors)
        self       = <TemporaryDirectory 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'>
C:\Miniconda\envs\mlos\Lib\tempfile.py:934: in _rmtree
    _shutil.rmtree(name, onexc=onexc)
        cls        = <class 'tempfile.TemporaryDirectory'>
        ignore_errors = False
        name       = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
        onexc      = <function TemporaryDirectory._rmtree.<locals>.onexc at 0x000001E9E466A2A0>
        repeated   = False
C:\Miniconda\envs\mlos\Lib\shutil.py:781: in rmtree
    return _rmtree_unsafe(path, onexc)
        dir_fd     = None
        ignore_errors = False
        onerror    = None
        onexc      = <function TemporaryDirectory._rmtree.<locals>.onexc at 0x000001E9E466A2A0>
        path       = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
C:\Miniconda\envs\mlos\Lib\shutil.py:635: in _rmtree_unsafe
    onexc(os.unlink, fullname, err)
        dirnames   = []
        dirpath    = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
        filenames  = ['mlos_bench.sqlite']
        fullname   = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'
        name       = 'mlos_bench.sqlite'
        onerror    = <function _rmtree_unsafe.<locals>.onerror at 0x000001E9E466A660>
        onexc      = <function TemporaryDirectory._rmtree.<locals>.onexc at 0x000001E9E466A2A0>
        path       = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
        results    = <generator object walk at 0x000001E9E2087CA0>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

func = <built-in function unlink>
path = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'
exc = PermissionError(13, 'The process cannot access the file because it is being used by another process')

    def onexc(func, path, exc):
        if isinstance(exc, PermissionError):
            if repeated and path == name:
                if ignore_errors:
                    return
                raise
    
            try:
                if path != name:
                    _resetperms(_os.path.dirname(path))
                _resetperms(path)
    
                try:
>                   _os.unlink(path)
E                   PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'

cls        = <class 'tempfile.TemporaryDirectory'>
exc        = PermissionError(13, 'The process cannot access the file because it is being used by another process')
func       = <built-in function unlink>
ignore_errors = False
name       = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio'
path       = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpuwby3eio\\mlos_bench.sqlite'
repeated   = False

C:\Miniconda\envs\mlos\Lib\tempfile.py:909: PermissionError

@bpkroth can you please take a look? Thank you!

[bpkroth]

Yeah, since this is just for test loading the configs, I switched it to using an in memory storage.
For later scheduler tests I'll want to use a file based one so we can check parallelism, but I can do that part later.