Skip to content

Commit e5314f2

Browse files
committed
refactor: use pytest tmp_path fixture to isolate test file operations
This change: - Replaces hardcoded directories with pytest's tmp_path fixture - Eliminates filesystem pollution between tests - Enables safe test parallelization - Improves path validation and error messages - Removes redundant cleanup code - Makes tests follow pytest best practices
1 parent 9b7c809 commit e5314f2

File tree

5 files changed

+77
-94
lines changed

5 files changed

+77
-94
lines changed

tests/custom/conftest.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,11 @@ def get_humanloop_client() -> GetHumanloopClientFn:
8686
if not os.getenv("HUMANLOOP_API_KEY"):
8787
pytest.fail("HUMANLOOP_API_KEY is not set for integration tests")
8888

89-
def _get_humanloop_client(use_local_files: bool = False) -> Humanloop:
89+
def _get_humanloop_client(use_local_files: bool = False, local_files_directory: str = "humanloop") -> Humanloop:
9090
return Humanloop(
9191
api_key=os.getenv("HUMANLOOP_API_KEY"),
9292
use_local_files=use_local_files,
93+
local_files_directory=local_files_directory,
9394
)
9495

9596
return _get_humanloop_client

tests/custom/integration/test_overload.py renamed to tests/custom/integration/test_local_file_operations.py

Lines changed: 33 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,19 @@
11
from pathlib import Path
2-
import os
3-
import tempfile
42

53
import pytest
64

75
from humanloop.error import HumanloopRuntimeError
86
from tests.custom.types import GetHumanloopClientFn, SyncableFile
97

108

11-
@pytest.fixture
12-
def cleanup_local_files():
13-
"""Cleanup any locally synced files after tests"""
14-
yield
15-
local_dir = Path("humanloop")
16-
if local_dir.exists():
17-
import shutil
18-
19-
shutil.rmtree(local_dir)
20-
21-
229
def test_path_validation(
2310
get_humanloop_client: GetHumanloopClientFn,
2411
syncable_files_fixture: list[SyncableFile],
25-
cleanup_local_files,
12+
tmp_path: Path,
2613
):
27-
"""Test path validation rules for local file usage."""
14+
"""Test validation of path formats for local file operations."""
2815
# GIVEN a client with local files enabled and remote files pulled
29-
humanloop_client = get_humanloop_client(use_local_files=True)
16+
humanloop_client = get_humanloop_client(use_local_files=True, local_files_directory=str(tmp_path))
3017
humanloop_client.pull()
3118
test_file = syncable_files_fixture[0]
3219

@@ -79,10 +66,10 @@ def test_path_validation(
7966

8067
def test_local_file_call(
8168
get_humanloop_client: GetHumanloopClientFn,
82-
cleanup_local_files,
8369
sdk_test_dir: str,
70+
tmp_path: Path,
8471
):
85-
"""Test using a local file for call operations with the overloaded client."""
72+
"""Test calling the API with a local prompt file."""
8673
# GIVEN a local prompt file with proper system tag
8774
prompt_content = """---
8875
model: gpt-4o
@@ -102,16 +89,14 @@ def test_local_file_call(
10289
</system>
10390
"""
10491

105-
# Create local file structure
106-
humanloop_dir = Path("humanloop")
107-
humanloop_dir.mkdir(exist_ok=True)
92+
# Create local file structure in temporary directory
10893
test_path = f"{sdk_test_dir}/capital_prompt"
109-
file_path = humanloop_dir / f"{test_path}.prompt"
94+
file_path = tmp_path / f"{test_path}.prompt"
11095
file_path.parent.mkdir(parents=True, exist_ok=True)
11196
file_path.write_text(prompt_content)
11297

11398
# GIVEN a client with local files enabled
114-
client = get_humanloop_client(use_local_files=True)
99+
client = get_humanloop_client(use_local_files=True, local_files_directory=str(tmp_path))
115100

116101
# WHEN calling the API with the local file path (without extension)
117102
response = client.prompts.call(
@@ -133,10 +118,10 @@ def test_local_file_call(
133118

134119
def test_local_file_log(
135120
get_humanloop_client: GetHumanloopClientFn,
136-
cleanup_local_files,
137121
sdk_test_dir: str,
122+
tmp_path: Path,
138123
):
139-
"""Test using a local file for log operations with the overloaded client."""
124+
"""Test logging data with a local prompt file."""
140125
# GIVEN a local prompt file with proper system tag
141126
prompt_content = """---
142127
model: gpt-4o
@@ -155,16 +140,14 @@ def test_local_file_log(
155140
</system>
156141
"""
157142

158-
# Create local file structure
159-
humanloop_dir = Path("humanloop")
160-
humanloop_dir.mkdir(exist_ok=True)
143+
# Create local file structure in temporary directory
161144
test_path = f"{sdk_test_dir}/geography_prompt"
162-
file_path = humanloop_dir / f"{test_path}.prompt"
145+
file_path = tmp_path / f"{test_path}.prompt"
163146
file_path.parent.mkdir(parents=True, exist_ok=True)
164147
file_path.write_text(prompt_content)
165148

166149
# GIVEN a client with local files enabled
167-
client = get_humanloop_client(use_local_files=True)
150+
client = get_humanloop_client(use_local_files=True, local_files_directory=str(tmp_path))
168151

169152
# GIVEN message content to log
170153
test_messages = [{"role": "user", "content": "What is the capital of France?"}]
@@ -189,16 +172,17 @@ def test_local_file_log(
189172
def test_overload_version_environment_handling(
190173
get_humanloop_client: GetHumanloopClientFn,
191174
syncable_files_fixture: list[SyncableFile],
175+
tmp_path: Path,
192176
):
193-
"""Test that overload_with_local_files correctly handles version_id and environment parameters."""
177+
"""Test handling of version_id and environment parameters with local files."""
194178
# GIVEN a client with use_local_files=True and pulled files
195-
humanloop_client = get_humanloop_client(use_local_files=True)
179+
humanloop_client = get_humanloop_client(use_local_files=True, local_files_directory=str(tmp_path))
196180
humanloop_client.pull()
197181

198182
# GIVEN a test file that exists locally
199183
test_file = syncable_files_fixture[0]
200184
extension = f".{test_file.type}"
201-
local_path = Path("humanloop") / f"{test_file.path}{extension}"
185+
local_path = tmp_path / f"{test_file.path}{extension}"
202186

203187
# THEN the file should exist locally
204188
assert local_path.exists(), f"Expected pulled file at {local_path}"
@@ -257,10 +241,10 @@ def test_overload_version_environment_handling(
257241

258242
# def test_agent_local_file_usage(
259243
# get_humanloop_client: GetHumanloopClientFn,
260-
# cleanup_local_files,
261244
# sdk_test_dir: str,
245+
# tmp_path: Path,
262246
# ):
263-
# """Test using a local agent file for call operations with the overloaded client."""
247+
# """Test using a local agent file for API calls."""
264248
# # NOTE: This test has been disabled as it fails intermittently in automated test runs
265249
# # but works correctly when tested manually. The issue appears to be related to test
266250
# # environment differences rather than actual code functionality.
@@ -279,54 +263,52 @@ def test_overload_version_environment_handling(
279263
# endpoint: chat
280264
# tools: []
281265
# ---
282-
266+
#
283267
# <system>
284268
# You are a helpful agent that provides concise answers. When asked about capitals of countries,
285269
# you respond with just the capital name, lowercase, with no punctuation or additional text.
286270
# </system>
287271
# """
288-
289-
# # Create local file structure
290-
# humanloop_dir = Path("humanloop")
291-
# humanloop_dir.mkdir(exist_ok=True)
272+
#
273+
# # Create local file structure in temporary directory
292274
# test_path = f"{sdk_test_dir}/capital_agent"
293-
# file_path = humanloop_dir / f"{test_path}.agent"
275+
# file_path = tmp_path / f"{test_path}.agent"
294276
# file_path.parent.mkdir(parents=True, exist_ok=True)
295277
# file_path.write_text(agent_content)
296-
278+
#
297279
# # GIVEN a client with local files enabled
298-
# client = get_humanloop_client(use_local_files=True)
299-
280+
# client = get_humanloop_client(use_local_files=True, local_files_directory=str(tmp_path))
281+
#
300282
# # WHEN calling the API with the local file path (without extension)
301283
# response = client.agents.call(
302284
# path=test_path, messages=[{"role": "user", "content": "What is the capital of France?"}]
303285
# )
304-
286+
#
305287
# # THEN the response should be successful
306288
# assert response is not None
307289
# assert response.logs is not None
308290
# assert len(response.logs) > 0
309-
291+
#
310292
# # AND the response should contain the expected output format (lowercase city name)
311293
# assert "paris" in response.logs[0].output.lower()
312-
294+
#
313295
# # AND the agent used should match our expected path
314296
# assert response.agent is not None
315297
# assert response.agent.path == test_path
316-
298+
#
317299
# # WHEN logging with the local agent file
318300
# test_messages = [{"role": "user", "content": "What is the capital of Germany?"}]
319301
# test_output = "Berlin is the capital of Germany."
320302
# log_response = client.agents.log(path=test_path, messages=test_messages, output=test_output)
321-
303+
#
322304
# # THEN the log should be successful
323305
# assert log_response is not None
324306
# assert log_response.agent_id is not None
325307
# assert log_response.id is not None # log ID
326-
308+
#
327309
# # WHEN retrieving the logged agent details
328310
# agent_details = client.agents.get(id=log_response.agent_id)
329-
311+
#
330312
# # THEN the details should match our expected path
331313
# assert agent_details is not None
332314
# assert test_path in agent_details.path

tests/custom/integration/test_sync.py

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,19 @@
99
def test_pull_basic(
1010
syncable_files_fixture: list[SyncableFile],
1111
get_humanloop_client: GetHumanloopClientFn,
12+
tmp_path: Path,
1213
):
13-
"""Test that humanloop.sync() correctly syncs remote files to local filesystem"""
14+
"""Test basic file syncing from remote to local filesystem."""
1415
# GIVEN a set of files in the remote system (from syncable_files_fixture)
15-
humanloop_client = get_humanloop_client()
16+
humanloop_client = get_humanloop_client(local_files_directory=str(tmp_path))
1617

1718
# WHEN running the sync
1819
humanloop_client.pull()
1920

2021
# THEN our local filesystem should mirror the remote filesystem in the HL Workspace
2122
for file in syncable_files_fixture:
2223
extension = f".{file.type}"
23-
local_path = Path("humanloop") / f"{file.path}{extension}"
24+
local_path = tmp_path / f"{file.path}{extension}"
2425

2526
# THEN the file and its directory should exist
2627
assert local_path.exists(), f"Expected synced file at {local_path}"
@@ -34,9 +35,10 @@ def test_pull_basic(
3435
def test_pull_with_invalid_path(
3536
get_humanloop_client: GetHumanloopClientFn,
3637
sdk_test_dir: str,
38+
tmp_path: Path,
3739
):
38-
"""Test that humanloop.sync() raises an error when the path is invalid"""
39-
humanloop_client = get_humanloop_client()
40+
"""Test error handling when path doesn't exist."""
41+
humanloop_client = get_humanloop_client(local_files_directory=str(tmp_path))
4042
non_existent_path = f"{sdk_test_dir}/non_existent_directory"
4143

4244
# Note: This test currently relies on the specific error message from list_files().
@@ -47,9 +49,10 @@ def test_pull_with_invalid_path(
4749

4850
def test_pull_with_invalid_environment(
4951
get_humanloop_client: GetHumanloopClientFn,
52+
tmp_path: Path,
5053
):
51-
"""Test that humanloop.sync() raises an error when the environment is invalid"""
52-
humanloop_client = get_humanloop_client()
54+
"""Test error handling when environment doesn't exist."""
55+
humanloop_client = get_humanloop_client(local_files_directory=str(tmp_path))
5356
with pytest.raises(HumanloopRuntimeError, match="Environment .* does not exist"):
5457
humanloop_client.pull(environment="invalid_environment")
5558

@@ -58,7 +61,7 @@ def test_pull_with_invalid_environment(
5861
# get_humanloop_client: GetHumanloopClientFn,
5962
# syncable_files_fixture: list[SyncableFile],
6063
# ):
61-
# """Test that humanloop.sync() correctly syncs files from a specific environment"""
64+
# """Test pulling files filtered by a specific environment."""
6265
# # NOTE: This test is currently not feasible to implement because:
6366
# # 1. We have no way of deploying to an environment using its name, only by ID
6467
# # 2. There's no API endpoint to retrieve environments for an organization
@@ -75,24 +78,22 @@ def test_pull_with_path_filter(
7578
get_humanloop_client: GetHumanloopClientFn,
7679
syncable_files_fixture: list[SyncableFile],
7780
sdk_test_dir: str,
81+
tmp_path: Path,
7882
):
79-
"""Test that humanloop.sync() correctly filters files by path when pulling"""
83+
"""Test that filtering by path correctly limits which files are synced."""
8084
# GIVEN a client
81-
humanloop_client = get_humanloop_client()
82-
83-
# First clear any existing files to ensure clean state
84-
import shutil
85-
86-
if Path("humanloop").exists():
87-
shutil.rmtree("humanloop")
85+
humanloop_client = get_humanloop_client(local_files_directory=str(tmp_path))
8886

8987
# WHEN pulling only files from the sdk_test_dir path
9088
humanloop_client.pull(path=sdk_test_dir)
9189

9290
# THEN count the total number of files synced
9391
synced_file_count = 0
94-
for path in Path("humanloop").glob("**/*"):
92+
for path in tmp_path.glob("**/*"):
9593
if path.is_file():
94+
# Check that the file is not empty
95+
content = path.read_text()
96+
assert content, f"File at {path} should not be empty"
9697
synced_file_count += 1
9798

9899
# The count should match our fixture length

0 commit comments

Comments
 (0)