Skip to content

Commit 17ac341

Browse files
committed
fix: resolve SQLite database file locking on Windows for unittests
- Add teardown_sqlalchemy() to properly dispose engine and release file locks - Fix clear_db() to dispose its engine in a finally block - Update all test teardowns to use teardown_sqlalchemy() - Add ignore_errors=True to shutil.rmtree calls as safety net - Add windows-latest to CI test matrix - Use poetry run instead of source $VENV for cross-platform CI Closes #291
1 parent 566c1c2 commit 17ac341

17 files changed

Lines changed: 67 additions & 37 deletions

File tree

.github/workflows/test.yml

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ jobs:
3939
strategy:
4040
fail-fast: true
4141
matrix:
42-
os: [ "ubuntu-latest", "macos-latest" ]
42+
os: [ "ubuntu-latest", "macos-latest", "windows-latest" ]
4343
python-version: [ "3.10", "3.11" ]
4444
defaults:
4545
run:
@@ -101,14 +101,12 @@ jobs:
101101
#----------------------------------------------
102102
- name: Run tests
103103
run: |
104-
source $VENV
105-
coverage run -m unittest discover -s tests
104+
poetry run coverage run -m unittest discover -s tests
106105
- name: Generate coverage report
107106
if: always()
108107
continue-on-error: true
109108
run: |
110-
source $VENV
111-
coverage xml
109+
poetry run coverage xml
112110
#----------------------------------------------
113111
# upload coverage stats
114112
#----------------------------------------------
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
from .sql_alchemy import Session, setup_sqlalchemy, SQLBaseModel, \
2-
create_all_tables, clear_db, SqliteDecimal
2+
create_all_tables, clear_db, teardown_sqlalchemy, SqliteDecimal
33

44
__all__ = [
55
"Session",
66
"setup_sqlalchemy",
77
"SQLBaseModel",
88
"create_all_tables",
99
"clear_db",
10+
"teardown_sqlalchemy",
1011
"SqliteDecimal"
1112
]

investing_algorithm_framework/infrastructure/database/sql_alchemy.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
from sqlalchemy import create_engine, StaticPool, String
55
from sqlalchemy import inspect
6-
from sqlalchemy.orm import DeclarativeBase, sessionmaker
6+
from sqlalchemy.orm import DeclarativeBase, sessionmaker, close_all_sessions
77
from sqlalchemy import TypeDecorator
88

99
from investing_algorithm_framework.domain import SQLALCHEMY_DATABASE_URI, \
@@ -83,6 +83,21 @@ def create_all_tables():
8383
SQLBaseModel.metadata.create_all(bind=Session().bind)
8484

8585

86+
def teardown_sqlalchemy():
87+
"""
88+
Dispose the engine and close all sessions to release file locks.
89+
This is essential on Windows where file locks are mandatory and
90+
prevent deletion of SQLite database files while connections are open.
91+
"""
92+
close_all_sessions()
93+
bind = Session.kw.get("bind")
94+
95+
if bind is not None:
96+
bind.dispose()
97+
98+
Session.configure(bind=None)
99+
100+
86101
from sqlalchemy import event
87102
from sqlalchemy.orm import mapper
88103
from datetime import timezone
@@ -98,6 +113,7 @@ def clear_db(db_uri):
98113
Returns:
99114
None
100115
"""
116+
engine = None
101117
# Drop all tables before deleting file
102118
try:
103119
engine = create_engine(db_uri)
@@ -107,12 +123,9 @@ def clear_db(db_uri):
107123
SQLBaseModel.metadata.drop_all(bind=engine)
108124
except Exception as e:
109125
logger.error(f"Error dropping tables: {e}")
110-
111-
# # Clear mappers (if using classical mappings)
112-
# try:
113-
# clear_mappers()
114-
# except Exception:
115-
# pass # ignore if not needed
126+
finally:
127+
if engine is not None:
128+
engine.dispose()
116129

117130

118131
@event.listens_for(mapper, "load")

tests/app/algorithm/test_run_strategy.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from investing_algorithm_framework import create_app, TradingStrategy, \
99
TimeUnit, PortfolioConfiguration, RESOURCE_DIRECTORY, Algorithm, \
1010
MarketCredential
11+
from investing_algorithm_framework.infrastructure.database import \
12+
teardown_sqlalchemy
1113
from tests.resources import random_string, OrderExecutorTest, \
1214
PortfolioProviderTest
1315

@@ -73,6 +75,7 @@ def setUp(self) -> None:
7375

7476
def tearDown(self) -> None:
7577
super().tearDown()
78+
teardown_sqlalchemy()
7679
database_dir = os.path.join(self.resource_dir, "databases")
7780
if os.path.exists(database_dir):
7881
shutil.rmtree(database_dir, ignore_errors=True)

tests/app/algorithm/test_trade_price_update.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
TimeUnit, PortfolioConfiguration, RESOURCE_DIRECTORY, \
99
MarketCredential, DataSource, INDEX_DATETIME, DataType, \
1010
CSVOHLCVDataProvider, BacktestDateRange
11+
from investing_algorithm_framework.infrastructure.database import \
12+
teardown_sqlalchemy
1113
from tests.resources import random_string, \
1214
PortfolioProviderTest, OrderExecutorTest
1315

@@ -55,6 +57,7 @@ def setUp(self) -> None:
5557

5658
def tearDown(self) -> None:
5759
super().tearDown()
60+
teardown_sqlalchemy()
5861
database_dir = os.path.join(self.resource_dir, "databases")
5962
if os.path.exists(database_dir):
6063
shutil.rmtree(database_dir, ignore_errors=True)

tests/app/backtesting/test_run_backtest.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from investing_algorithm_framework import create_app, RESOURCE_DIRECTORY, \
99
TradingStrategy, PortfolioConfiguration, TimeUnit, Algorithm, \
1010
BacktestDateRange
11+
from investing_algorithm_framework.infrastructure.database import \
12+
teardown_sqlalchemy
1113

1214

1315
class TestStrategy(TradingStrategy):
@@ -47,12 +49,13 @@ def setUp(self) -> None:
4749

4850
def tearDown(self) -> None:
4951
super().tearDown()
52+
teardown_sqlalchemy()
5053

5154
if os.path.exists(self.backtest_databases_directory):
52-
shutil.rmtree(self.backtest_databases_directory)
55+
shutil.rmtree(self.backtest_databases_directory, ignore_errors=True)
5356

5457
if os.path.exists(self.backtest_report_directory):
55-
shutil.rmtree(self.backtest_report_directory)
58+
shutil.rmtree(self.backtest_report_directory, ignore_errors=True)
5659

5760
def test_report_creation(self):
5861
app = create_app(

tests/app/test_backtesting.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
from investing_algorithm_framework import TradingStrategy, Algorithm, \
1919
create_app, RESOURCE_DIRECTORY, PortfolioConfiguration, \
2020
BacktestDateRange, TimeUnit
21+
from investing_algorithm_framework.infrastructure.database import \
22+
teardown_sqlalchemy
2123
from investing_algorithm_framework.domain import SQLALCHEMY_DATABASE_URI
2224

2325

@@ -70,13 +72,14 @@ def setUp(self) -> None:
7072

7173
def tearDown(self) -> None:
7274
super().tearDown()
75+
teardown_sqlalchemy()
7376
for path in (
7477
os.path.join(self.resource_dir, "databases"),
7578
self.backtest_databases_dir,
7679
self.backtest_reports_dir,
7780
):
7881
if os.path.exists(path):
79-
shutil.rmtree(path)
82+
shutil.rmtree(path, ignore_errors=True)
8083

8184

8285
# ---------------------------------------------------------------------------
@@ -253,4 +256,3 @@ def test_run_backtests(self):
253256
risk_free_rate=0.027
254257
)
255258
self.assertEqual(3, len(reports))
256-

tests/app/test_eventloop.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ def tearDown(self) -> None:
240240
)
241241

242242
if os.path.exists(databases_directory):
243-
shutil.rmtree(databases_directory)
243+
shutil.rmtree(databases_directory, ignore_errors=True)
244244

245245
if os.path.exists(backtest_databases_directory):
246-
shutil.rmtree(backtest_databases_directory)
246+
shutil.rmtree(backtest_databases_directory, ignore_errors=True)

tests/app/test_start.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
from investing_algorithm_framework import create_app, TradingStrategy, \
88
TimeUnit, RESOURCE_DIRECTORY, PortfolioConfiguration, Algorithm, \
99
MarketCredential
10+
from investing_algorithm_framework.infrastructure.database import \
11+
teardown_sqlalchemy
1012
from tests.resources import OrderExecutorTest, PortfolioProviderTest
1113

1214

@@ -106,6 +108,7 @@ def setUp(self) -> None:
106108
)
107109

108110
def tearDown(self):
111+
teardown_sqlalchemy()
109112
for subdir in ("databases", "backtest_databases"):
110113
path = os.path.join(self.resource_dir, subdir)
111114
if os.path.exists(path):
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"algorithm_id": "TestStrategy"
2+
"algorithm_id": "BacktestTestStrategy"
33
}

0 commit comments

Comments
 (0)