Skip to content

test: run all tests on a single fact instance #1330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Molter73 wants to merge 4 commits into
base: mauro/fix/cleanup-lpm
Choose a base branch
from
Draft

test: run all tests on a single fact instance #1330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b8ae0e5
test: speed up integration tests with session-scoped fact container
Molter73 Jul 29, 2026
d87800c
test: add MetricsSnapshot for delta-based metric assertions
Molter73 Jul 29, 2026
91c63d2
test: decouple session-scoped fixtures from EventServer
Molter73 Jul 30, 2026
9afbb7a
chore: better log time slice for tests
Molter73 Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fact/src/config/reloader/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl Reloader {
let path = PathBuf::from(file);
if path.exists() {
let mtime = match path.metadata() {
Ok(m) => m.mtime(),
Ok(m) => m.mtime_nsec(),
Err(e) => {
warn!("Failed to stat {file}: {e}");
warn!("Configuration reloading may not work");
Expand Down
271 changes: 173 additions & 98 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import json
import os
import time
from shutil import rmtree
from tempfile import NamedTemporaryFile, mkdtemp
from time import sleep
Expand All @@ -13,11 +14,26 @@
import requests
import yaml

from metrics import MetricsSnapshot
from server import EventServer, GrpcServer, OtlpServer

# Declare files holding fixtures
pytest_plugins = ['test_editors.commons']

ENDPOINT_ADDRESS = '127.0.0.1:9000'


def base_config() -> dict:
return {
'paths': [],
'endpoint': {
'address': ENDPOINT_ADDRESS,
'expose_metrics': True,
'health_check': True,
},
'json': True,
'scan_interval': 0,
}


@pytest.fixture
def monitored_dir():
Expand All @@ -33,7 +49,7 @@ def monitored_dir():
@pytest.fixture
def test_file(monitored_dir: str):
"""
Create a temporary file for tests
Create a temporary file for tests.

This file needs to exist when fact starts up for the inode tracking
algorithm to work.
Expand Down Expand Up @@ -77,21 +93,23 @@ def _get_output_modes(config: pytest.Config) -> list[str]:


def pytest_generate_tests(metafunc: pytest.Metafunc):
if 'server' in metafunc.fixturenames:
if 'server' in metafunc.definition._fixtureinfo.argnames:
modes = _get_output_modes(metafunc.config)
metafunc.parametrize('server', modes, indirect=True)


@pytest.fixture
@pytest.fixture(scope='session')
def server(request: pytest.FixtureRequest):
"""
Start and stop an event server.

Parameterised via --output to create either a GrpcServer or an
OtlpServer. When --output=all, every test that uses this fixture
runs once per output mode.
OtlpServer. When --output=all, every test that directly
requests this fixture runs once per output mode. Tests that
only get it transitively (via autouse ``fact_config``) default
to grpc.
"""
mode = request.param
mode = getattr(request, 'param', 'grpc')
if mode == 'otlp':
s: EventServer = OtlpServer()
else:
Expand Down Expand Up @@ -143,99 +161,40 @@ def dump_container_inspect(
json.dump(container_inspect, f, indent=2)


@pytest.fixture
def fact_config(
request: pytest.FixtureRequest,
monitored_dir: str,
logs_dir: str,
server: EventServer,
):
cwd = os.getcwd()
config: dict = {
'paths': [
f'{monitored_dir}',
f'{monitored_dir}/**/*',
'/mounted/**/*',
'/container-dir/**/*',
],
'endpoint': {
'address': '127.0.0.1:9000',
'expose_metrics': True,
'health_check': True,
},
'json': True,
'scan_interval': 0,
}

if server.output_mode == 'otlp':
config['otel'] = {'endpoint': 'http://127.0.0.1:4318/v1/logs'}
else:
config['grpc'] = {'url': 'http://127.0.0.1:9999'}
@pytest.fixture(scope='session')
def fact_config_file():
"""
Session-scoped config file shared across all tests.

config_file = NamedTemporaryFile( # noqa: SIM115
The file is created once with a baseline config and rewritten
by the per-test ``fact_config`` fixture before each test.
"""
cwd = os.getcwd()
config = base_config()
f = NamedTemporaryFile( # noqa: SIM115
prefix='fact-config-',
suffix='.yml',
dir=cwd,
mode='w',
)
yaml.dump(config, config_file)

yield config, config_file.name
with (
open(os.path.join(logs_dir, 'fact.yml'), 'w') as f,
open(config_file.name) as r,
):
f.write(r.read())
config_file.close()


@pytest.fixture
def test_container(
request: pytest.FixtureRequest,
docker_client: docker.DockerClient,
monitored_dir: str,
ignored_dir: str,
):
"""
Run a container for triggering events in.
"""
container = docker_client.containers.run(
'quay.io/fedora/fedora:43',
detach=True,
tty=True,
volumes={
ignored_dir: {
'bind': '/mounted',
'mode': 'z',
},
monitored_dir: {
'bind': '/unmonitored',
'mode': 'z',
},
},
name='fedora',
)
container.exec_run('mkdir /container-dir')

yield container
yaml.dump(config, f)
yield f.name
f.close()
Comment on lines +172 to +182

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Flush the config file before it's consumed by the container.

yaml.dump(config, f) writes into NamedTemporaryFile's buffered stream, but f is never flushed before yield f.name — the very path bind-mounted read-only into the fact container at startup (lines 218-222). If the write hasn't hit the OS buffer yet when Docker reads the file, the container can start with an empty/truncated config, breaking session bootstrap intermittently.

🐛 Proposed fix 
     yaml.dump(config, f)
+    f.flush()
     yield f.name
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cwd = os.getcwd()
config = base_config()
f = NamedTemporaryFile( # noqa: SIM115
prefix='fact-config-',
suffix='.yml',
dir=cwd,
mode='w',
)
yaml.dump(config, config_file)
yield config, config_file.name
with (
open(os.path.join(logs_dir, 'fact.yml'), 'w') as f,
open(config_file.name) as r,
):
f.write(r.read())
config_file.close()
@pytest.fixture
def test_container(
request: pytest.FixtureRequest,
docker_client: docker.DockerClient,
monitored_dir: str,
ignored_dir: str,
):
"""
Run a container for triggering events in.
"""
container = docker_client.containers.run(
'quay.io/fedora/fedora:43',
detach=True,
tty=True,
volumes={
ignored_dir: {
'bind': '/mounted',
'mode': 'z',
},
monitored_dir: {
'bind': '/unmonitored',
'mode': 'z',
},
},
name='fedora',
)
container.exec_run('mkdir /container-dir')
yield container
yaml.dump(config, f)
yield f.name
f.close()
cwd = os.getcwd()
config = base_config()
f = NamedTemporaryFile( # noqa: SIM115
prefix='fact-config-',
suffix='.yml',
dir=cwd,
mode='w',
)
yaml.dump(config, f)
f.flush()
yield f.name
f.close()
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/conftest.py` around lines 172 - 182, Flush the NamedTemporaryFile
stream after yaml.dump(config, f) and before yielding f.name in the fixture,
ensuring the container reads the complete configuration when it starts. Keep the
existing temporary-file lifecycle and cleanup unchanged.


container.stop(timeout=1)
container.remove()


@pytest.fixture(autouse=True)
@pytest.fixture(scope='session', autouse=True)
def fact(
request: pytest.FixtureRequest,
docker_client: docker.DockerClient,
fact_config: tuple[dict, str],
server: EventServer,
logs_dir: str,
test_file: str,
fact_config_file: str,
):
"""
Run the fact docker container for integration tests.
Session-scoped fact container shared across all tests.

The container is started once and kept alive for the entire test
session. Between tests the ``fact_config`` fixture rewrites the
config file and sends SIGHUP so that fact picks up new paths.
"""
config, config_file = fact_config
image = request.config.getoption('--image')
assert isinstance(image, str)
container = docker_client.containers.run(
Expand All @@ -256,20 +215,21 @@ def fact(
'bind': '/host',
'mode': 'ro',
},
config_file: {
fact_config_file: {
'bind': '/etc/stackrox/fact.yml',
'mode': 'ro',
},
},
)

container_log = os.path.join(logs_dir, 'fact.log')
# Wait for container to be ready
session_logs = os.path.join(os.getcwd(), 'logs')
container_log = os.path.join(session_logs, 'fact.log')

os.makedirs(session_logs, exist_ok=True)

for _ in range(10):
try:
resp = requests.get(
f'http://{config["endpoint"]["address"]}/health_check'
)
resp = requests.get(f'http://{ENDPOINT_ADDRESS}/health_check')
if resp.status_code == 200:
break
except (requests.RequestException, requests.ConnectionError) as e:
Expand All @@ -283,26 +243,141 @@ def fact(

yield container

# Capture prometheus metrics before stopping the container
if config['endpoint']['expose_metrics']:
metric_log = os.path.join(logs_dir, 'metrics')
resp = requests.get(f'http://{config["endpoint"]["address"]}/metrics')
try:
resp = requests.get(f'http://{ENDPOINT_ADDRESS}/metrics')
if resp.status_code == 200:
with open(metric_log, 'w') as f:
with open(os.path.join(session_logs, 'metrics'), 'w') as f:
f.write(resp.text)
except requests.RequestException:
pass

container.stop(timeout=5)
exit_status = container.wait(timeout=2)
try:
dump_logs(container, container_log)
dump_container_inspect(
docker_client, container, os.path.join(logs_dir, 'container.json')
docker_client,
container,
os.path.join(session_logs, 'container.json'),
)
finally:
container.remove()
assert exit_status['StatusCode'] == 0


def reload_fact(
container: docker.models.containers.Container,
config: dict,
config_file: str,
delay: float = 0.1,
):
with open(config_file, 'w') as f:
yaml.dump(config, f)
container.kill('SIGHUP')
sleep(delay)


@pytest.fixture(autouse=True)
def fact_config(
fact: docker.models.containers.Container,
fact_config_file: str,
monitored_dir: str,
server: EventServer,
logs_dir: str,
test_file: str,
):
"""
Per-test config that hot-reloads fact via SIGHUP.

On setup: writes a config with the test's ``monitored_dir`` paths,
drains stale events from the server queue, and signals fact to
reload.

On teardown: restores the baseline config (empty paths) so that
fact does not fire on directory cleanup, and captures
timestamp-sliced container logs for the test.
"""
test_start = time.time()

config = base_config()
if server.output_mode == 'otlp':
config['otel'] = {'endpoint': 'http://127.0.0.1:4318/v1/logs'}
else:
config['grpc'] = {'url': 'http://127.0.0.1:9999'}
config['paths'] = [
monitored_dir,
f'{monitored_dir}/**/*',
'/mounted/**/*',
'/container-dir/**/*',
]

server.drain()
reload_fact(fact, config, fact_config_file)

metrics_before = MetricsSnapshot.fetch(ENDPOINT_ADDRESS)

yield config, fact_config_file

Comment on lines +300 to +320

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Unguarded metrics_before fetch is inconsistent with the guarded teardown fetch.

metrics_before = MetricsSnapshot.fetch(ENDPOINT_ADDRESS) at line 317 has no exception handling, while the analogous metrics_after fetch at teardown (lines 327-333) is wrapped in try/except Exception: pass. A transient failure reaching /metrics right after the SIGHUP-triggered reload would error out the autouse fixture and fail every test, whereas the same failure at teardown is silently swallowed. Consider wrapping the setup fetch symmetrically (or making both paths consistently strict) with None handling in delta()/get().

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/conftest.py` around lines 300 - 320, Make the setup metrics collection
around MetricsSnapshot.fetch(ENDPOINT_ADDRESS) consistent with the teardown path
by handling transient fetch exceptions and representing an unavailable snapshot
as None. Update the corresponding delta()/get() usage to safely handle None
while preserving normal metrics comparisons when both snapshots are available.

with (
open(os.path.join(logs_dir, 'fact.yml'), 'w') as out,
open(fact_config_file) as src,
):
out.write(src.read())

try:
metrics_after = MetricsSnapshot.fetch(ENDPOINT_ADDRESS)
metrics_delta = metrics_before.delta(metrics_after)
with open(os.path.join(logs_dir, 'metrics'), 'w') as f:
f.write(metrics_delta.to_text())
except Exception:
pass

try:
test_end = time.time()
log_bytes = fact.logs(since=test_start, until=test_end)
with open(os.path.join(logs_dir, 'fact.log'), 'wb') as f:
f.write(log_bytes)
except Exception:
pass

config = base_config()
reload_fact(fact, config, fact_config_file)


@pytest.fixture
def test_container(
request: pytest.FixtureRequest,
docker_client: docker.DockerClient,
monitored_dir: str,
ignored_dir: str,
):
"""
Run a container for triggering events in.
"""
container = docker_client.containers.run(
'quay.io/fedora/fedora:43',
detach=True,
tty=True,
volumes={
ignored_dir: {
'bind': '/mounted',
'mode': 'z',
},
monitored_dir: {
'bind': '/unmonitored',
'mode': 'z',
},
},
name='fedora',
)
container.exec_run('mkdir /container-dir')

yield container

container.stop(timeout=1)
container.remove()


def pytest_addoption(parser: pytest.Parser):
parser.addoption(
'--image',
Expand Down
Loading
Loading