Skip to content

Commit 6a90d27

Browse files
emielveremielver
authored
Add integration tests + add support for filters in percentile metrics (#35)
1 parent 74fd8bd commit 6a90d27

File tree

7 files changed

+266
-5
lines changed

7 files changed

+266
-5
lines changed

‎.github/workflows/run_tests.yml‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
workflow_dispatch:
88

99
jobs:
10-
test:
10+
test_functionality:
1111
runs-on: ubuntu-latest
1212

1313
steps:
@@ -26,4 +26,26 @@ jobs:
2626
2727
- name: Run Pytest
2828
run: |
29-
pytest tests
29+
pytest tests
30+
31+
test_installation:
32+
runs-on: ubuntu-latest
33+
needs: test_functionality
34+
35+
steps:
36+
- name: Checkout code
37+
uses: actions/checkout@v3
38+
39+
- name: Set up Python
40+
uses: actions/setup-python@v4
41+
with:
42+
python-version: "3.11"
43+
44+
- name: Install pytest
45+
run: |
46+
python -m pip install --upgrade pip
47+
pip install pytest
48+
49+
- name: Run Installation Test
50+
run: |
51+
pytest tests/test_integration.py::test_package_installation_and_dependencies -v

‎Makefile‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
make test:
2+
@if [ ! -d ".venv" ]; then \
3+
echo "Creating virtual environment..."; \
4+
python3 -m venv .venv; \
5+
fi
6+
@echo "Activating virtual environment and installing dependencies..."
7+
@. .venv/bin/activate && \
8+
pip freeze > /tmp/requirements_to_uninstall.txt && \
9+
pip uninstall -y -r /tmp/requirements_to_uninstall.txt && \
10+
pip install -r requirements.txt && \
11+
pytest tests

‎eppo_metrics_sync/schema/eppo_metric_schema.json‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
"description": "An optional identifier for the source of metrics. Recommended to be defined in CI/CD and not in yaml files",
1010
"type": "string"
1111
},
12+
"reference_url": {
13+
"description": "An optional URL to link to in the Eppo UI",
14+
"type": "string"
15+
},
1216
"fact_sources": {
1317
"description": "A fact source, corresponds to a fact SQL definition in the Eppo UI",
1418
"type": "array",
@@ -29,7 +33,7 @@
2933
"type": "string"
3034
},
3135
"reference_url": {
32-
"description": "URL to use for 'Open in GitHub' in Eppo UI (optional)",
36+
"description": "An optional URL to link to in the Eppo UI",
3337
"type": "string"
3438
},
3539
"entities": {
@@ -149,7 +153,7 @@
149153
"type": "number"
150154
},
151155
"reference_url": {
152-
"description": "URL to use for 'Open in GitHub' in Eppo UI (optional)",
156+
"description": "An optional URL to link to in the Eppo UI",
153157
"type": "string"
154158
},
155159
"guardrail_cutoff": {
@@ -251,6 +255,14 @@
251255
"winsorization_upper_percentile": {
252256
"description": "Percentile at which to clip aggregated metrics (optional)",
253257
"type": "number"
258+
},
259+
"winsor_lower_fixed_value": {
260+
"description": "A fixed value to clip the lower percentile at (optional)",
261+
"type": "number"
262+
},
263+
"winsor_upper_fixed_value": {
264+
"description": "A fixed value to clip the upper percentile at (optional)",
265+
"type": "number"
254266
}
255267
}
256268
},
@@ -306,13 +318,25 @@
306318
"description": "What time unit to use: minutes, hours, days, or weeks (optional)",
307319
"enum": ["minutes", "hours", "days", "weeks", "calendar_days"]
308320
},
321+
"aggregation_enable_aging_subject_filter": {
322+
"description": "Controls whether subjects (entities) should be filtered out of metric calculations until they have 'aged' for a sufficient period. This is particularly important for metrics that require a certain observation period to be meaningful.",
323+
"type": "boolean"
324+
},
309325
"winsorization_lower_percentile": {
310326
"description": "Percentile at which to clip aggregated metrics (optional)",
311327
"type": "number"
312328
},
313329
"winsorization_upper_percentile": {
314330
"description": "Percentile at which to clip aggregated metrics (optional)",
315331
"type": "number"
332+
},
333+
"winsor_lower_fixed_value": {
334+
"description": "A fixed value to clip the lower percentile at (optional)",
335+
"type": "number"
336+
},
337+
"winsor_upper_fixed_value": {
338+
"description": "A fixed value to clip the upper percentile at (optional)",
339+
"type": "number"
316340
}
317341
}
318342
},
@@ -331,6 +355,32 @@
331355
"type": "number",
332356
"minimum": 0,
333357
"maximum": 1
358+
},
359+
"filters": {
360+
"description": "Optional fact property filters to apply",
361+
"type": "array",
362+
"items": {
363+
"type": "object",
364+
"additionalProperties": false,
365+
"required": ["fact_property", "operation", "values"],
366+
"properties": {
367+
"fact_property": {
368+
"description": "Must match one of the values in fact_sources.properties.name",
369+
"type": "string"
370+
},
371+
"operation": {
372+
"description": "Either equals or not_equals",
373+
"enum": ["equals", "not_equals"]
374+
},
375+
"values": {
376+
"description": "Values to include if operation is 'equals', or exclude if operation is 'not_equals'",
377+
"type": "array",
378+
"items": {
379+
"type": "string"
380+
}
381+
}
382+
}
383+
}
334384
}
335385
}
336386
}

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
44

55
[project]
66
name = "eppo_metrics_sync"
7-
version = "0.1.8"
7+
version = "0.1.9"
88
description = "Sync metrics to Eppo"
99
readme = "README.md"
1010
requires-python = ">=3.7"

‎tests/test_integration.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import os
2+
import subprocess
3+
import tempfile
4+
from pathlib import Path
5+
import venv
6+
7+
def test_package_installation_and_dependencies():
8+
"""
9+
Test that the package can be installed in a fresh virtual environment
10+
and all dependencies are compatible.
11+
"""
12+
# Create a temporary directory for the virtual environment
13+
with tempfile.TemporaryDirectory() as venv_dir:
14+
# Create a virtual environment
15+
venv.create(venv_dir, with_pip=True)
16+
17+
# Determine the path to the Python executable in the virtual environment
18+
python_executable = os.path.join(venv_dir, 'bin', 'python')
19+
20+
# Get the absolute path to the project root (parent directory of the tests directory)
21+
project_root = str(Path(__file__).parent.parent.absolute())
22+
23+
# Install the package in development mode
24+
install_result = subprocess.run(
25+
[python_executable, '-m', 'pip', 'install', '-e', project_root],
26+
stdout=subprocess.PIPE,
27+
stderr=subprocess.PIPE,
28+
text=True
29+
)
30+
31+
assert install_result.returncode == 0, f"Failed to install package: {install_result.stderr}"
32+
33+
# Try to import and run a basic command to verify functionality
34+
run_result = subprocess.run(
35+
[python_executable, '-m', 'eppo_metrics_sync', '--help'],
36+
stdout=subprocess.PIPE,
37+
stderr=subprocess.PIPE,
38+
text=True
39+
)
40+
41+
assert run_result.returncode == 0, f"Failed to run package: {run_result.stderr}"
42+
assert "usage:" in run_result.stdout, "Help text not found in output"
43+
44+
# Check if we can use the basic functionality (dry run with empty dir)
45+
with tempfile.TemporaryDirectory() as empty_dir:
46+
basic_run = subprocess.run(
47+
[python_executable, '-m', 'eppo_metrics_sync', empty_dir, '--dryrun'],
48+
stdout=subprocess.PIPE,
49+
stderr=subprocess.PIPE,
50+
text=True
51+
)
52+
53+
# We consider the test passed if it can be installed and run
54+
# Even if it returns an error code for an empty directory
55+
assert True

‎tests/yaml/valid/all_api_fields.yml‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
reference_url: https://github.com/Eppo-exp/eppo-metrics-sync
2+
fact_sources:
3+
- name: Video Streaming Source
4+
sql: |
5+
SELECT
6+
events.*
7+
FROM customer_db.streaming.events as events
8+
WHERE event_type = 'VideoPlay'
9+
timestamp_column: TS
10+
entities:
11+
- entity_name: User
12+
column: USER_ID
13+
facts:
14+
- name: Video View # facts without column specified reflect "EACH RECORD"
15+
- name: Watch Duration
16+
column: DURATION_SEC
17+
properties:
18+
- name: Device Type
19+
column: DEVICE_TYPE
20+
include_experiment_computation: true
21+
- name: Content Category
22+
column: CONTENT_CATEGORY
23+
include_experiment_computation: false
24+
- name: Subscription Tier
25+
column: SUBSCRIPTION_TIER
26+
include_experiment_computation: true
27+
reference_url: https://github.com/Eppo-exp/eppo-metrics-sync
28+
metrics:
29+
- name: Unique Videos Viewed
30+
entity: User # it would be nice if this was optional if there is exactly 1 entity defined above
31+
is_guardrail: true
32+
type: simple
33+
numerator:
34+
fact_name: Video View
35+
operation: distinct_entity
36+
- name: Average Watch Time (Winsorized)
37+
entity: User
38+
type: ratio
39+
numerator:
40+
fact_name: Watch Duration
41+
operation: sum
42+
winsor_upper_fixed_value: 7200
43+
winsor_lower_fixed_value: 0
44+
denominator:
45+
fact_name: Video View
46+
operation: distinct_entity
47+
winsor_lower_fixed_value: 0
48+
winsor_upper_fixed_value: 1000
49+
aggregation_enable_aging_subject_filter: true
50+
- name: Mobile Premium Content Views
51+
entity: User
52+
metric_display_style: decimal
53+
type: simple
54+
numerator:
55+
fact_name: Video View
56+
operation: sum
57+
filters:
58+
- fact_property: Device Type
59+
operation: equals
60+
values:
61+
- "MOBILE"
62+
- fact_property: Subscription Tier
63+
operation: equals
64+
values:
65+
- "PREMIUM"
66+
- name: Last Watch Duration (Winsorized)
67+
entity: User
68+
type: simple
69+
description: The last video watch duration with winsorization
70+
numerator:
71+
fact_name: Watch Duration
72+
operation: last_value
73+
winsorization_lower_percentile: 0.05
74+
winsorization_upper_percentile: 0.95
75+
- name: Watch Duration (p95)
76+
entity: User
77+
type: percentile
78+
description: 95th percentile of watch duration
79+
metric_display_style: decimal
80+
minimum_detectable_effect: 0.05
81+
percentile:
82+
fact_name: Watch Duration
83+
percentile_value: 0.95
84+
filters:
85+
- fact_property: Device Type
86+
operation: equals
87+
values:
88+
- "MOBILE"

‎tests/yaml/valid/percentile_with_filters.yaml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
fact_sources:
2+
- name: App Usage With Filters
3+
sql: |
4+
SELECT
5+
timestamp as TS,
6+
user_id,
7+
app_open_duration,
8+
app_type
9+
FROM app_usage_table
10+
timestamp_column: TS
11+
entities:
12+
- entity_name: User
13+
column: user_id
14+
facts:
15+
- name: App Open With Type
16+
column: app_open_duration
17+
properties:
18+
- name: app_type
19+
column: app_type
20+
metrics:
21+
- name: Mobile App Opens (p90)
22+
description: 90th percentile of mobile app opens
23+
type: percentile
24+
entity: User
25+
metric_display_style: decimal
26+
minimum_detectable_effect: 0.05
27+
reference_url: ""
28+
guardrail_cutoff: null
29+
percentile:
30+
fact_name: App Open With Type
31+
percentile_value: 0.90
32+
filters:
33+
- fact_property: app_type
34+
operation: equals
35+
values: ["mobile"]

0 commit comments

Comments
 (0)