Skip to content

Commit 0771719

Browse files
authored
Baseline test improvements: pretty-print snapshots, unified-diff failures, docstrings, local data (#140)
Sets a cleaner baseline for the test suite so future data and code PRs are easier to develop, review, and triage. All changes confined to `tests/` (plus a one-character typo fix in `database/etfs.csv`). **All 32 tests pass.** ## Snapshot diffs that actually tell you what changed The `test_show_options*.json` snapshots are JSON arrays of category labels (countries, sectors, currencies, …) — single-line strings of 1-2 KB. When a database update added/removed one entry, the GitHub diff was one giant string change with no way to see what differed. ```diff - string_value = json.dumps(data, **kwargs) + string_value = json.dumps(data, indent=2, ensure_ascii=False, **kwargs) ``` `ensure_ascii=False` also means non-ASCII characters render natively (`Côte d'Ivoire` instead of escape sequences). ## Unified-diff on snapshot mismatch `Recorder.assert_equal()` previously raised a `AssertionError: Change detected` with two truncated 500-char strings, often appearing identical in the visible part even when they actually differed later. We hit this exact issue while developing #141 and #143. Now `assert_equal()` produces a proper `difflib.unified_diff` showing which line(s) changed; `assert_in_list` gained a proper assertion message. ## Tests now read the PR branch's data, not main The library defaults to fetching `compression/*` from the GitHub `main` branch over HTTP. This meant tests on a PR validated the data on `main`, not the data in the PR — silently passing data-breaking PRs and failing on data-fixing PRs. Two cooperating fixes: - Every `tests/test_*.py` now instantiates with `use_local_location=True` so the library reads the local checkout. - `tests/conftest.py` regenerates `compression/*.bz2` and `compression/categories/*.gzip` from the checked-out `database/*.csv` at import time, mirroring the production `database_update.yml` workflow. This must run *before* pytest collects the test modules (test-module imports instantiate `fd.X(use_local_location=True)`), so it is a top-level statement, not a fixture. The compression artifacts themselves are not committed in this PR — they are deterministically derived from the CSVs and the test suite regenerates them on every run. ## Test module quality (incidental cleanup) Since the regen touched every test file, took the opportunity to: - Fix wrong module docstrings (`test_equities.py` and `test_moneymarkets.py` both started with `"""Currencies Test Module"""`). - Replace `# pylint: disable=missing-function-docstring` with real docstrings on every test function in the 7 asset-class files and in `test_sec_enrichment_controller.py`. - Add type hints `(recorder: Recorder) -> None`; `Recorder` import under `TYPE_CHECKING`, guarded by `from __future__ import annotations` — zero runtime cost. ## conftest.py cleanup - Removed `# pylint: skip-file` so static analysis is no longer suppressed. - Cast `request.config.getoption("--rewrite-expected")` return through `bool(...)` to satisfy the declared `-> bool` annotation (previously `Any | None`). ## Test infrastructure config (pyproject.toml) - Dropped `pytest-recording` from dev deps — unused; the real dep used by `conftest.py` is `pytest-recorder` (which provides the `record_mode` / `disable_recording` fixtures). - Declared `[tool.pytest.ini_options]` `testpaths = ["tests"]`, `addopts = "--strict-markers"`, and the `record_stdout` marker so future PRs can't silently introduce undeclared markers. ## Side fix: ASYMshsare → ASYMshares (etfs.csv + tests) While running the new local-data tests we noticed `ASPY` (Leverage Shares ASYMmetric 500 ETF) had `family = "ASYMshsare"` — a typo in `database/etfs.csv`. Corrected to `ASYMshares` along with the references in `tests/test_etfs.py` and the affected fixture snapshots. The maintainer's `Update Compression Files` workflow will regenerate the binary artifacts on `main` post-merge. ## What this PR explicitly does NOT do Deferred to focused follow-ups (out of scope here): - `@pytest.mark.parametrize` / class-based grouping — would change pytest node IDs and break `Recorder.capture()` positional snapshot naming (renames 90+ files in one go). - `pytest-xdist` parallel execution — adds a new dev dep; pure perf win. - Wiring `pytest-cov` (already in dev deps) into CI for coverage reporting. - Filling the 22% coverage gap on `financedatabase/helpers.py` (`FinanceFrame.to_toolkit`, `show_options` URL error path, `case_sensitive=True`).
1 parent 725fc05 commit 0771719

68 files changed

Lines changed: 3792 additions & 109 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

database/etfs.csv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2275,7 +2275,7 @@ ASIL-GB.AQ,MULTI UNITS LUXEMB,GBP,"MULTI UNITS LUXEMB is an investment fund inco
22752275
ASIL.L,Multi Units Luxembourg - Lyxor China Enterprise (HSCEI) UCITS ETF,GBP,"Multi Units Luxembourg - Lyxor China Enterprise (HSCEI) UCITS ETF is an exchange-traded fund designed to track the performance of the Hang Seng China Enterprises Index (HSCEI). The ETF provides exposure to the largest companies in China that are listed on the Hong Kong Stock Exchange. Lyxor Asset Management, a subsidiary of Societe Generale, manages the ETF, which is domiciled in Luxembourg.",Equities,,Lyxor International Asset Management,LSE,
22762276
ASIU.L,Lyxor China Enterprise (HSCEI) UCITS ETF - Acc-EUR,USD,"The investment objective of the MULTI UNITS LUXEMBOURG — Lyxor China Enterprise (HSCEI) UCITS ETF (the ""Sub-Fund"") is to track both the upward and the downward evolution of the Hang Seng China Enterprises Index Net Total Return index (the ""Index"") denominated in Hong Kong dollars (HKD), and representative of the major Chinese equities known as ""H-shares, while minimizing the volatility of the difference between the return of the Sub-Fund and the return of the Index (the ""Tracking Error"").",Equities,,Lyxor International Asset Management,LSE,LU1900068914
22772277
ASPY,ASYMshares ASYMmetric 500 ETF,USD,"The investment seeks to track the total return performance, before fees and expenses, of the ASYMmetric 500 Index.
2278-
The index is a rules-based, quantitative long/short hedging strategy that seeks to provide protection against bear market losses and to capture the majority of bull market gains with respect to exposure to the 500 largest capitalized equity securities publicly traded in the United States, which is referred to as the index's ""market"". Under normal market conditions, the fund will invest at least 80% of its total assets in securities and cash included in the index's Long Book.",Equities,Equities,ASYMshsare,PCX,
2278+
The index is a rules-based, quantitative long/short hedging strategy that seeks to provide protection against bear market losses and to capture the majority of bull market gains with respect to exposure to the 500 largest capitalized equity securities publicly traded in the United States, which is referred to as the index's ""market"". Under normal market conditions, the fund will invest at least 80% of its total assets in securities and cash included in the index's Long Book.",Equities,Equities,ASYMshares,PCX,
22792279
ASR0.BE,BNPP ENERGY TRAN. CLCAP,EUR,"BNPP ENERGY TRAN. CLCAP is a fund managed by BNP Paribas Asset Management focused on investing in companies involved in the energy transition. The ""CLCAP"" indicates that it is an accumulation share class, meaning dividends are reinvested. Further information is required to understand the fund's specific investment strategy and the sectors within the energy transition it targets.",Energy,,BNP Paribas Asset Management,BER,
22802280
ASR0.MU,Parvest SICAV - Energy Innovators,EUR,"Parvest SICAV - Energy Innovators is an investment fund focused on companies involved in the energy transition, including renewable energy, energy efficiency, and related technologies. The fund aims to provide long-term capital appreciation by investing in companies that are contributing to a more sustainable energy future. BNP Paribas Asset Management manages the fund, which is part of the Parvest SICAV fund range domiciled in Luxembourg.",Energy,,BNP Paribas Asset Management,MUN,
22812281
ASR1.BE,BNPP GER.MF EQ. CL.CAP,EUR,"BNPP GER.MF EQ. CL.CAP is a fund managed by BNP Paribas Asset Management that invests in German equities (EQ). ""MF"" suggests it might be a multi-factor fund. ""CL.CAP"" indicates an capitalization-weighted accumulation share class, where dividends are reinvested.",Equities,,BNP Paribas Asset Management,BER,

poetry.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pyproject.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,9 @@ disable_error_code = "misc, valid-type, attr-defined, index"
9191
[tool.pytest.ini_options]
9292
filterwarnings = [
9393
"ignore::pytest.PytestAssertRewriteWarning:",
94+
]
95+
testpaths = ["tests"]
96+
addopts = "--strict-markers"
97+
markers = [
98+
"record_stdout: capture and snapshot stdout output for the marked test",
9499
]

tests/conftest.py

Lines changed: 110 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import difflib
12
import json
23
import os
34
import pathlib
@@ -28,7 +29,88 @@
2829
"txt": [str],
2930
}
3031

31-
# pylint: skip-file
32+
REPO_ROOT = pathlib.Path(__file__).resolve().parents[1]
33+
ASSET_CSVS = {
34+
"cryptos": "summary",
35+
"currencies": "summary",
36+
"equities": "summary",
37+
"etfs": "summary",
38+
"funds": "summary",
39+
"indices": "summary",
40+
"moneymarkets": "summary",
41+
}
42+
43+
44+
def _regenerate_compression_artifacts() -> None:
45+
"""Mirror the Database-Update workflow so tests run against compression
46+
artifacts derived from the *checked-out* CSV files.
47+
48+
The financedatabase library reads `compression/<asset>.bz2` and
49+
`compression/categories/<asset>_categories.gzip`; without this step those
50+
artifacts lag the `database/*.csv` on a PR branch and tests silently
51+
validate against `main` instead of the PR change.
52+
"""
53+
compression_dir = REPO_ROOT / "compression"
54+
categories_dir = compression_dir / "categories"
55+
for asset, name_col in ASSET_CSVS.items():
56+
csv_path = REPO_ROOT / "database" / f"{asset}.csv"
57+
if not csv_path.exists():
58+
continue
59+
df = pd.read_csv(csv_path)
60+
df.to_csv(compression_dir / f"{asset}.bz2", index=False, compression="bz2")
61+
indexed = pd.read_csv(csv_path, index_col=0)
62+
categories: dict[str, Any] = {}
63+
skip_cols = {"name", name_col, "manager_name", "manager_bio"}
64+
for column in indexed.columns:
65+
if column in skip_cols:
66+
continue
67+
vals = indexed[column].dropna().unique()
68+
vals.sort()
69+
categories[column] = vals
70+
cat_df = pd.DataFrame.from_dict(categories, orient="index").reset_index()
71+
cat_df.to_csv(
72+
categories_dir / f"{asset}_categories.gzip",
73+
index=False,
74+
compression="gzip",
75+
)
76+
77+
78+
def _snapshot_compression_artifacts() -> dict[pathlib.Path, bytes]:
79+
"""Capture current bytes of compression artifacts so they can be restored."""
80+
compression_dir = REPO_ROOT / "compression"
81+
snapshot: dict[pathlib.Path, bytes] = {}
82+
for asset in ASSET_CSVS:
83+
for path in (
84+
compression_dir / f"{asset}.bz2",
85+
compression_dir / "categories" / f"{asset}_categories.gzip",
86+
):
87+
if path.exists():
88+
snapshot[path] = path.read_bytes()
89+
return snapshot
90+
91+
92+
def _restore_compression_artifacts(snapshot: dict[pathlib.Path, bytes]) -> None:
93+
"""Restore compression artifacts to their pre-test contents."""
94+
for path, data in snapshot.items():
95+
try:
96+
path.write_bytes(data)
97+
except Exception:
98+
pass
99+
100+
101+
# Regenerate compression artifacts BEFORE pytest collects test modules. The
102+
# asset-class test modules instantiate `fd.X(use_local_location=True)` at
103+
# import time, so the artifacts must already be in sync with the checked-out
104+
# CSVs by then — a session-scoped fixture runs too late. A snapshot of the
105+
# original bytes is taken first; `pytest_sessionfinish` restores them so the
106+
# working tree stays clean for contributors.
107+
_compression_snapshot = _snapshot_compression_artifacts()
108+
_regenerate_compression_artifacts()
109+
110+
111+
def pytest_sessionfinish(session, exitstatus) -> None: # noqa: ARG001
112+
"""Restore compression artifacts after the pytest session ends."""
113+
_restore_compression_artifacts(_compression_snapshot)
32114

33115

34116
class Record:
@@ -43,7 +125,7 @@ def extract_string(data: Any, **kwargs) -> str:
43125
**kwargs,
44126
)
45127
elif isinstance(data, tuple(EXTENSIONS_MATCHING["json"])):
46-
string_value = json.dumps(data, **kwargs)
128+
string_value = json.dumps(data, indent=2, ensure_ascii=False, **kwargs)
47129
else:
48130
raise AttributeError(f"Unsupported type : {type(data)}")
49131

@@ -225,19 +307,39 @@ def assert_equal(self):
225307

226308
for record in record_list:
227309
if record.record_changed:
310+
expected = (record.recorded or "").splitlines()
311+
actual = (record.captured or "").splitlines()
312+
diff = "\n".join(
313+
difflib.unified_diff(
314+
expected,
315+
actual,
316+
fromfile="expected",
317+
tofile="actual",
318+
lineterm="",
319+
n=3,
320+
)
321+
)
322+
if not diff:
323+
diff = (
324+
f"(line-level diff empty — content differs only in trailing whitespace or EOL)\n"
325+
f"expected length={len(record.recorded or '')}, "
326+
f"actual length={len(record.captured or '')}"
327+
)
228328
raise AssertionError(
229-
"Change detected\n"
230-
f"Record : {record.record_path}\n"
231-
f"Expected : {record.recorded[:self.display_limit]}\n"
232-
f"Actual : {record.captured[:self.display_limit]}\n"
329+
f"Snapshot mismatch: {record.record_path}\n"
330+
f"Run pytest with --rewrite-expected to regenerate, or inspect the diff below.\n\n"
331+
f"{diff}"
233332
)
234333

235334
def assert_in_list(self, in_list: list[str]):
236335
record_list = self.__record_list
237336

238337
for record in record_list:
239338
for string_value in in_list:
240-
assert string_value in record.captured # noqa
339+
assert string_value in record.captured, ( # noqa: S101
340+
f"Expected substring {string_value!r} not found in "
341+
f"{record.record_path}"
342+
)
241343

242344
def persist(self):
243345
record_list = self.__record_list
@@ -345,7 +447,7 @@ def pytest_addoption(parser: Parser):
345447
@pytest.fixture(scope="session") # type: ignore
346448
def rewrite_expected(request: SubRequest) -> bool:
347449
"""Force rewriting of all expected data by : `record_stdout` and `recorder`."""
348-
return request.config.getoption("--rewrite-expected")
450+
return bool(request.config.getoption("--rewrite-expected"))
349451

350452

351453
@pytest.fixture
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
symbol,name,currency,summary,category_group,category,family,exchange,isin
22
ASPY,ASYMshares ASYMmetric 500 ETF,USD,"The investment seeks to track the total return performance, before fees and expenses, of the ASYMmetric 500 Index.
3-
The index is a rules-based, quantitative long/short hedging strategy that seeks to provide protection against bear market losses and to capture the majority of bull market gains with respect to exposure to the 500 largest capitalized equity securities publicly traded in the United States, which is referred to as the index&#39;s ""market"". Under normal market conditions, the fund will invest at least 80% of its total assets in securities and cash included in the index&#39;s Long Book.",Equities,Equities,ASYMshsare,PCX,
3+
The index is a rules-based, quantitative long/short hedging strategy that seeks to provide protection against bear market losses and to capture the majority of bull market gains with respect to exposure to the 500 largest capitalized equity securities publicly traded in the United States, which is referred to as the index&#39;s ""market"". Under normal market conditions, the fund will invest at least 80% of its total assets in securities and cash included in the index&#39;s Long Book.",Equities,Equities,ASYMshares,PCX,
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
symbol,name,currency,summary,category_group,category,family,exchange,isin
22
ASPY,ASYMshares ASYMmetric 500 ETF,USD,"The investment seeks to track the total return performance, before fees and expenses, of the ASYMmetric 500 Index.
3-
The index is a rules-based, quantitative long/short hedging strategy that seeks to provide protection against bear market losses and to capture the majority of bull market gains with respect to exposure to the 500 largest capitalized equity securities publicly traded in the United States, which is referred to as the index&#39;s ""market"". Under normal market conditions, the fund will invest at least 80% of its total assets in securities and cash included in the index&#39;s Long Book.",Equities,Equities,ASYMshsare,PCX,
3+
The index is a rules-based, quantitative long/short hedging strategy that seeks to provide protection against bear market losses and to capture the majority of bull market gains with respect to exposure to the 500 largest capitalized equity securities publicly traded in the United States, which is referred to as the index&#39;s ""market"". Under normal market conditions, the fund will invest at least 80% of its total assets in securities and cash included in the index&#39;s Long Book.",Equities,Equities,ASYMshares,PCX,
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
1-
["cryptocurrency", "currency"]
1+
[
2+
"cryptocurrency",
3+
"currency"
4+
]

0 commit comments

Comments
 (0)