Skip to content

Commit daf17c4

Browse files
committed
ci: DRY test env configuration in PR workflow
- Extracted env var configuration into configure_test_env() function. - Ensures rollback stage uses same config as PR test stage (fixes missing PEER_DISCOVERY and other vars in rollback). - Document testing patterns learned during hex symbol migration
1 parent ba9a457 commit daf17c4

File tree

2 files changed

+234
-11
lines changed

2 files changed

+234
-11
lines changed

.github/workflows/pr-preprod-tests.yaml

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,19 @@ jobs:
5858
fi
5959
}
6060
61-
ensure_var REMOVE_SPENT_UTXOS false
62-
ensure_var REMOVE_SPENT_UTXOS_LAST_BLOCKS_GRACE_COUNT 129600
63-
ensure_var DB_PORT 5433
64-
ensure_var TOKEN_REGISTRY_ENABLED true
65-
ensure_var TOKEN_REGISTRY_BASE_URL http://preview.integrations.cf-systems.internal:8080/api
66-
ensure_var TOKEN_REGISTRY_CACHE_TTL_HOURS 1
67-
ensure_var TOKEN_REGISTRY_LOGO_FETCH true
68-
ensure_var TOKEN_REGISTRY_REQUEST_TIMEOUT_SECONDS 2
69-
ensure_var PEER_DISCOVERY true
61+
configure_test_env() {
62+
ensure_var REMOVE_SPENT_UTXOS false
63+
ensure_var REMOVE_SPENT_UTXOS_LAST_BLOCKS_GRACE_COUNT 129600
64+
ensure_var DB_PORT 5433
65+
ensure_var TOKEN_REGISTRY_ENABLED true
66+
ensure_var TOKEN_REGISTRY_BASE_URL http://preview.integrations.cf-systems.internal:8080/api
67+
ensure_var TOKEN_REGISTRY_CACHE_TTL_HOURS 1
68+
ensure_var TOKEN_REGISTRY_LOGO_FETCH true
69+
ensure_var TOKEN_REGISTRY_REQUEST_TIMEOUT_SECONDS 2
70+
ensure_var PEER_DISCOVERY true
71+
}
72+
73+
configure_test_env
7074
7175
7276
- name: Build and start services with PR code
@@ -275,7 +279,29 @@ jobs:
275279
if: failure() && (steps.test.outcome == 'failure' || steps.test.outcome == 'cancelled')
276280
run: |
277281
cd /home/integration/git/cardano-rosetta-java
278-
sed -i "s#^DB_PORT=.*#DB_PORT=5433#" .env.docker-compose-preprod
282+
283+
ensure_var() {
284+
local key="$1"
285+
local value="$2"
286+
local file=".env.docker-compose-preprod"
287+
if grep -q "^${key}=" "$file"; then
288+
sed -i "s#^${key}=.*#${key}=${value}#" "$file"
289+
else
290+
echo "${key}=${value}" >> "$file"
291+
fi
292+
}
293+
294+
configure_test_env() {
295+
ensure_var REMOVE_SPENT_UTXOS false
296+
ensure_var REMOVE_SPENT_UTXOS_LAST_BLOCKS_GRACE_COUNT 129600
297+
ensure_var DB_PORT 5433
298+
ensure_var TOKEN_REGISTRY_ENABLED true
299+
ensure_var TOKEN_REGISTRY_BASE_URL http://preview.integrations.cf-systems.internal:8080/api
300+
ensure_var TOKEN_REGISTRY_CACHE_TTL_HOURS 1
301+
ensure_var TOKEN_REGISTRY_LOGO_FETCH true
302+
ensure_var TOKEN_REGISTRY_REQUEST_TIMEOUT_SECONDS 2
303+
ensure_var PEER_DISCOVERY true
304+
}
279305
280306
echo "⚠️ Test failed - stopping services"
281307
@@ -346,7 +372,9 @@ jobs:
346372
LAST_TAG=$(git tag --sort=-version:refname | head -1)
347373
echo "Rolling back to stable version: $LAST_TAG"
348374
git checkout -f $LAST_TAG
349-
sed -i "s#^DB_PORT=.*#DB_PORT=5433#" .env.docker-compose-preprod
375+
376+
# Re-apply test configuration for stable version
377+
configure_test_env
350378
351379
docker compose \
352380
--env-file .env.docker-compose-preprod \
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
# Testing Guidelines
2+
3+
Hard-earned lessons from writing integration tests for Rosetta API.
4+
5+
## Test Structure
6+
7+
### AAA Pattern (Arrange-Act-Assert)
8+
9+
Separate phases visually with a blank line. No phase comments - structure should be obvious.
10+
11+
```python
12+
def test_currency_filter_returns_matching_transactions(...):
13+
# Data gathering
14+
asset = network_data["assets"][0]
15+
response = client.search_transactions(currency={...})
16+
txs = response.json()["transactions"]
17+
18+
# Validation
19+
assert len(txs) > 0
20+
for tx in txs:
21+
assert asset["symbol_hex"] in currencies_from_tx
22+
```
23+
24+
### Filter Semantics
25+
26+
When testing filters, validate **ALL** results match the filter criteria, not "at least one".
27+
28+
```python
29+
# WRONG - only checks first match
30+
found = False
31+
for item in results:
32+
if matches_filter(item):
33+
found = True
34+
break
35+
assert found
36+
37+
# RIGHT - validates every result
38+
for item in results:
39+
assert matches_filter(item), "Filter should return only matching items"
40+
```
41+
42+
### Collection Pattern
43+
44+
Use "collect then assert membership" instead of "find first and break".
45+
46+
```python
47+
# Cleaner, no nested breaks
48+
currencies_in_tx = [
49+
token["currency"]["symbol"].lower()
50+
for op in tx["operations"]
51+
if "tokenBundle" in op.get("metadata", {})
52+
for bundle in op["metadata"]["tokenBundle"]
53+
for token in bundle.get("tokens", [])
54+
]
55+
assert expected_symbol in currencies_in_tx
56+
```
57+
58+
## Cardano-Specific
59+
60+
### Native Assets Live in TokenBundle
61+
62+
In `/search/transactions` responses, native assets are ALWAYS in `operations[].metadata.tokenBundle[].tokens[]`, never in `operations[].amount.currency` (which is always ADA).
63+
64+
```python
65+
# Native assets sit in UTXOs alongside ADA
66+
for op in tx["operations"]:
67+
if "metadata" in op and "tokenBundle" in op["metadata"]:
68+
for bundle in op["metadata"]["tokenBundle"]:
69+
for token in bundle.get("tokens", []):
70+
currencies.append(token["currency"]["symbol"])
71+
```
72+
73+
### Currency Symbols Must Be Hex
74+
75+
After issue #610, all currency symbols for native assets must be hex-encoded.
76+
77+
```python
78+
# network_test_data.yaml
79+
assets:
80+
- symbol: "tTEURO" # Human-readable
81+
symbol_hex: "74544555524f" # Use this in tests
82+
```
83+
84+
## Helper Functions
85+
86+
### Helpers Return Data, Tests Assert
87+
88+
Data-fetching helpers should return `None` on failure and let tests decide if that's an error.
89+
90+
```python
91+
# GOOD - returns data or None
92+
def _fetch_token_from_account(...) -> Tuple[Dict | None, Dict | None]:
93+
response = client.account_balance(...)
94+
if response.status_code != 200:
95+
return None, None
96+
# ... find and return
97+
return currency, metadata
98+
99+
# In test - explicit assertion
100+
currency, metadata = _fetch_token_from_account(...)
101+
assert currency is not None, f"Token {token['ticker']} not found"
102+
```
103+
104+
### Universal Preconditions → Fixtures
105+
106+
Setup requirements shared across all tests should be pytest fixtures, not helper functions.
107+
108+
```python
109+
# GOOD - pytest fixture for universal setup
110+
@pytest.fixture
111+
def tokens_config(network_data):
112+
"""Extract and validate tokens_in_registry configuration."""
113+
tokens = network_data.get("tokens_in_registry")
114+
assert tokens, "network_test_data.yaml must define tokens_in_registry"
115+
return tokens
116+
117+
# Test signature declares dependencies
118+
def test_enrichment(client, network, tokens_config, has_token_registry):
119+
for token in tokens_config: # Fixture already validated
120+
...
121+
```
122+
123+
### Domain Assertion Helpers
124+
125+
Grouping related assertions is OK if the name is explicit about what it validates.
126+
127+
```python
128+
# Acceptable - domain-specific, clear name
129+
def _verify_all_metadata_fields_match(currency, metadata, token):
130+
"""Verify currency and enriched metadata fields match expected token configuration.
131+
132+
Validates:
133+
- currency: symbol_hex, decimals
134+
- metadata: policyId, subject, name, description, ticker, url
135+
"""
136+
assert metadata.get("policyId") == token["policy_id"]
137+
# ... 6 more assertions
138+
```
139+
140+
## Test Documentation
141+
142+
### Tests Are Documentation
143+
144+
Tests should be self-contained and readable in isolation. Favor inline clarity over DRY.
145+
146+
When debugging a failed test, you want to understand:
147+
- What was being checked?
148+
- How was it being checked?
149+
- Why did it fail?
150+
151+
Abstractions that hide this information make debugging harder.
152+
153+
### Docstrings for Helpers
154+
155+
Document what fails and why, not just what the function does.
156+
157+
```python
158+
def _fetch_peers_with_retry(...):
159+
"""Fetch peers from /network/status, retrying for peer discovery 5-min initial delay.
160+
161+
Returns: List of peers, or empty list if still not populated after 5 minutes.
162+
"""
163+
```
164+
165+
## Control Flow
166+
167+
### if = Business Rule, assert = Validation
168+
169+
```python
170+
# if - conditional logic (field is optional)
171+
if registry_value is not None:
172+
assert rosetta_value == registry_value # Validation: IF present, must match
173+
174+
# assert - requirement (field is mandatory)
175+
assert rosetta_value is not None # Fails if missing
176+
```
177+
178+
### Avoid Nested Breaks
179+
180+
Use collection patterns or early returns instead of multiple break statements.
181+
182+
## What NOT to Do
183+
184+
- ❌ Don't hide assertions in helper functions (Mystery Guest anti-pattern)
185+
- ❌ Don't mix data gathering and validation phases
186+
- ❌ Don't validate "at least one" when testing filters (validate ALL)
187+
- ❌ Don't add phase comments (`# Arrange`, `# Assert`) - structure should be obvious
188+
- ❌ Don't check operation amounts for native assets - check tokenBundle
189+
- ❌ Don't use ASCII symbols for currency filters - use hex encoding
190+
191+
## When in Doubt
192+
193+
Ask: "If this test fails, can I immediately see WHAT failed by reading the test code?"
194+
195+
If the answer requires jumping to helper functions, refactor to make failure points visible.

0 commit comments

Comments
 (0)