-
Notifications
You must be signed in to change notification settings - Fork 383
Snapshot record then reply testing #740
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,16 @@ | ||
| from typing import override | ||
| import functools | ||
| import inspect | ||
| import json | ||
| import os | ||
| from collections.abc import Callable, Coroutine | ||
| from typing import Any, override | ||
| from unittest.mock import patch | ||
| from urllib import parse | ||
|
|
||
| import vcr | ||
| from vcr.config import RecordMode | ||
| from vcr.request import Request | ||
|
|
||
| from splunklib.ai.model import PredefinedModel | ||
| from tests.ai_test_model import InternalAIModel, TestLLMSettings, create_model | ||
| from tests.testlib import SDKTestCase | ||
|
|
@@ -42,3 +54,149 @@ async def model(self) -> PredefinedModel: | |
| model = await create_model(self.test_llm_settings) | ||
| self._model = model | ||
| return model | ||
|
|
||
|
|
||
| def ai_snapshot_test() -> Callable[ | ||
| [Callable[..., Coroutine[Any, Any, None]]], Callable[..., Coroutine[Any, Any, None]] | ||
| ]: | ||
| def decorator( | ||
| fn: Callable[..., Coroutine[Any, Any, None]], | ||
| ) -> Callable[..., Coroutine[Any, Any, None]]: | ||
| source_file = inspect.getfile(fn) | ||
| test_dir = os.path.dirname(source_file) | ||
| test_file = os.path.splitext(os.path.basename(source_file))[0] | ||
|
|
||
| snapshot_dir = os.path.join(test_dir, "snapshots", test_file) | ||
| snapshot_filename = f"{fn.__qualname__}.json" | ||
|
|
||
| @functools.wraps(fn) | ||
| async def wrapper(self: AITestCase, *args: Any, **kwargs: Any) -> None: | ||
| settings = self.test_llm_settings | ||
| assert settings.internal_ai is not None | ||
|
|
||
| internal_ai_hostname = parse.urlparse( | ||
| settings.internal_ai.base_url | ||
| ).hostname | ||
| assert internal_ai_hostname is not None | ||
|
|
||
| REDACTED_APP_KEY = "[[[--APPKEY-REDACTED-]]]" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this could be moved to the top of file |
||
|
|
||
| class _JSONFriendlySerializer: | ||
| def deserialize(self, serialized: str) -> Any: | ||
| assert settings.internal_ai is not None | ||
| serialized = serialized.replace( | ||
| REDACTED_APP_KEY, settings.internal_ai.app_key | ||
| ) | ||
|
|
||
| data = json.loads(serialized) | ||
| for interaction in data.get("interactions", []): | ||
| interaction["request"]["uri"] = interaction["request"][ | ||
| "uri" | ||
| ].replace("internal-ai-host", internal_ai_hostname, 1) | ||
|
|
||
| interaction["request"]["body"] = json.dumps( | ||
| interaction["request"]["body"] | ||
| ) | ||
| body = interaction["response"]["body"] | ||
| interaction["response"]["body"] = {} | ||
| interaction["response"]["body"]["string"] = json.dumps(body) | ||
|
|
||
| return data | ||
|
|
||
| def serialize(self, dict: Any) -> str: | ||
| for interaction in dict.get("interactions", []): | ||
| interaction["request"]["uri"] = interaction["request"][ | ||
| "uri" | ||
| ].replace(internal_ai_hostname, "internal-ai-host", 1) | ||
|
|
||
| body = interaction["request"]["body"] | ||
| interaction["request"]["body"] = json.loads(body) | ||
|
|
||
| resp_body = interaction["response"]["body"]["string"] | ||
| interaction["response"]["body"] = json.loads(resp_body) | ||
|
|
||
| out = json.dumps(dict, indent=4) + "\n" | ||
| assert settings.internal_ai is not None | ||
| out = out.replace(settings.internal_ai.app_key, REDACTED_APP_KEY) | ||
|
|
||
| # Assert that nothing is leaking into the public snapshots. | ||
| assert internal_ai_hostname not in out.lower() | ||
| assert settings.internal_ai.app_key.lower() not in out.lower() | ||
| assert settings.internal_ai.base_url.lower() not in out.lower() | ||
| assert settings.internal_ai.token_url.lower() not in out.lower() | ||
| assert settings.internal_ai.client_id.lower() not in out.lower() | ||
| assert settings.internal_ai.client_secret.lower() not in out.lower() | ||
|
|
||
| return out | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious if we should look for the other secrets for our testing LLM before serializing - to make sure we do not leak anything by mistake when creating new snapshots. or add some ci stage for that. WDYT? |
||
|
|
||
| def _before_record_request(request: Request) -> Request | None: | ||
| url = parse.urlparse(request.uri) # pyright: ignore[reportUnknownArgumentType, reportUnknownVariableType] | ||
| if url.hostname == internal_ai_hostname: | ||
| request.headers = {} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we keep some specific headers for like Authorization and keep comparing them? |
||
| return request | ||
| return None | ||
|
|
||
| def _before_record_response(response: Any) -> Any: | ||
| response["headers"] = {} | ||
| return response | ||
|
|
||
| def _json_body_matcher(r1: Any, r2: Any) -> None: | ||
| b1 = json.loads(r1.body) | ||
| b2 = json.loads(r2.body) | ||
| if b1 != b2: | ||
| raise AssertionError(f"Body mismatch:\n{b1}\n!=\n{b2}") | ||
|
|
||
| my_vcr = vcr.VCR( | ||
| cassette_library_dir=snapshot_dir, | ||
| serializer="json-friendly", | ||
| record_mode=RecordMode.ONCE, | ||
| match_on=[ | ||
| "method", | ||
| "scheme", | ||
| "host", | ||
| "port", | ||
| "path", | ||
| "query", | ||
| "jsonbody", | ||
| ], | ||
| before_record_request=_before_record_request, | ||
| before_record_response=_before_record_response, | ||
| # record_on_exception=False, | ||
| # drop_unused_requests=True, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to be removed if not needed |
||
| ) | ||
| my_vcr.register_serializer("json-friendly", _JSONFriendlySerializer()) | ||
| my_vcr.register_matcher("jsonbody", _json_body_matcher) | ||
|
|
||
| with my_vcr.use_cassette(snapshot_filename): # pyright: ignore[reportGeneralTypeIssues] | ||
| await fn(self, *args, **kwargs) | ||
|
|
||
| return wrapper | ||
|
|
||
| return decorator | ||
|
|
||
|
|
||
| def deterministic_thread_ids() -> Callable[ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's cool, we don't need to change the test body to accommodate that. |
||
| [Callable[..., Coroutine[Any, Any, None]]], Callable[..., Coroutine[Any, Any, None]] | ||
| ]: | ||
| def decorator( | ||
| fn: Callable[..., Coroutine[Any, Any, None]], | ||
| ) -> Callable[..., Coroutine[Any, Any, None]]: | ||
| @functools.wraps(fn) | ||
| async def wrapper(self: AITestCase, *args: Any, **kwargs: Any) -> None: | ||
| counter = 0 | ||
|
|
||
| def _deterministic_uuid() -> str: | ||
| nonlocal counter | ||
| result = f"00000000-0000-0000-0000-{counter:012d}" | ||
| counter += 1 | ||
| return result | ||
|
|
||
| with patch( | ||
| "splunklib.ai.engines.langchain._thread_id_new_uuid", | ||
| side_effect=_deterministic_uuid, | ||
| ): | ||
| await fn(self, *args, **kwargs) | ||
|
|
||
| return wrapper | ||
|
|
||
| return decorator | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] the name suggests it can be used only for AI. What do you think about record_snapshot name?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do filtering here, so i think it only works for AI 😄 .