Conversation
tests/e2e/README.md
Outdated
| ```bash | ||
| e2e/ | ||
| ├── README.md # This documentation | ||
| ├── configs/ # Static agent configuration |
There was a problem hiding this comment.
Added README file
There was a problem hiding this comment.
It's good that you have all your e2e stuff together under its own directory.
| pexpect | ||
| pyhocon | ||
| pytest-xdist | ||
| pytest-timeout |
There was a problem hiding this comment.
Added requirement for e2e tests
There was a problem hiding this comment.
Should these requirements go to tests/e2e/requirements.txt then?
tests/e2e/conftest.py
Outdated
| # conftest.py | ||
| # ------------------------------------------------------------------------ | ||
| # Provides custom CLI flags, dynamic test generation, and environment setup. | ||
| # Pytest configuration to share like MusicNerdPro test |
There was a problem hiding this comment.
This file is triggered by pytest to set up the environment for the e2e test cases to run. It is located and should be in the e2e directory.
|
|
||
| # --- Step 5: Double-check that PID file is removed (optional cleanup) | ||
| assert not os.path.exists(PID_FILE), f"❌ [SERVER] PID file still exists: {PID_FILE}" | ||
| print(f"🧹 [SERVER] PID file successfully removed: {PID_FILE}") |
There was a problem hiding this comment.
Clean-up the PID file.
| @@ -0,0 +1,60 @@ | |||
| # tests/e2e/tools/stop_last_server.py | |||
|
|
|||
There was a problem hiding this comment.
Another option is to stop the PID listed in the file, which is started by the start server tool.
|
|
||
| # ------------------------------------------------------------------------ | ||
| # Shared Logging Setup for E2E Tests and CLI Runners | ||
| # ------------------------------------------------------------------------ |
| import os | ||
| from pyhocon import ConfigFactory | ||
|
|
||
| NAME_DATA_HOCON = "music_nerd_pro_data" |
There was a problem hiding this comment.
to loader music nerd pro hocon
| def extract_agent_response(output: str, start_marker="Response from MusicNerdPro:"): | ||
| """ | ||
| Extracts agent's response text based on a start marker. | ||
| Strips out trailing cost information or CLI prompts. |
There was a problem hiding this comment.
From the Music Nerd Pro agent output, CLI gets the message
| try: | ||
| parsed = json.loads(block.replace("'", '"')) | ||
| if "running_cost" in parsed: | ||
| return f"running_cost: {parsed['running_cost']}" |
There was a problem hiding this comment.
Loop Agent output to get COST output from CLI Music Nerd Pro
| # mnpt_runner.py | ||
| # ------------------------------------------------------------------------ | ||
| # CLI-based test runner: drives input/output to the MusicNerdPro agent CLI | ||
| # ------------------------------------------------------------------------ |
There was a problem hiding this comment.
This test's heart uses the agent_cli and simulates the user reading and typing input from the terminal.
| thinking_file_arg = build_thinking_file_arg(conn, repeat_index, use_thinking_file) | ||
|
|
||
| # Build command to launch agent CLI | ||
| command = ( |
There was a problem hiding this comment.
From this active terminal launch, the agent_cli
| logging.info(f"[TEST] CMD: {command}") | ||
|
|
||
| # Start the agent CLI process | ||
| child = pexpect.spawn(command, encoding="utf-8", echo=False) |
There was a problem hiding this comment.
Using the pexpect to capture the text from the terminal.
| """ | ||
| start = time.time() | ||
| while time.time() - start < timeout: | ||
| try: |
There was a problem hiding this comment.
This will ensure the process stops this proc
|
|
||
| def start_server(conn: str): | ||
| """ | ||
| Start the agent server as a detached background subprocess. |
| # After starting the subprocess and writing the PID | ||
| with open(PID_FILE, "a") as f: | ||
| f.write(f"{proc.pid}\n") | ||
| logging.info(f"📝 [SERVER] PID {proc.pid} written to {PID_FILE}") |
There was a problem hiding this comment.
write its PID to file
| def stop_all_servers(): | ||
| """ | ||
| Stop all PIDs listed in the PID file. Clean up the file. | ||
| """ |
There was a problem hiding this comment.
Stop all server according to PID's file.
|
|
||
|
|
||
| def stop_server_by_pid(pid: int): | ||
| try: |
There was a problem hiding this comment.
Stop sever by a pid id
|
|
||
|
|
||
| def _remove_pid_from_file(pid: int): | ||
| if not os.path.exists(PID_FILE): |
There was a problem hiding this comment.
Delete as clean up pip file
| @@ -0,0 +1,53 @@ | |||
| name: Smoke Test | |||
|
|
|||
There was a problem hiding this comment.
I added a new GitHub CRON Job smoke test trigger file, as @donn-leaf suggested.
.github/workflows/tests.yml
Outdated
|
|
||
| - name: Run Smoke Tests | ||
| run: | | ||
| python -m tests.e2e.tools.smoke_test_runner |
There was a problem hiding this comment.
Removed smoke test from building tests file.
|
|
||
| - name: Run pytest (excluding integration tests) | ||
| run: pytest --verbose -m "not integration" --timer-top-n 10 | ||
| - name: Run pytest Run All Other Tests (excluding integration and e2e) |
There was a problem hiding this comment.
This name seems redundant. How about just
Run pytest excluding integration, e2e and smoke tests
| - name: Run pytest (excluding integration tests) | ||
| run: pytest --verbose -m "not integration" --timer-top-n 10 | ||
| - name: Run pytest Run All Other Tests (excluding integration and e2e) | ||
| run: pytest --verbose -m "not integration and not smoke and not e2e" --timer-top-n 10 |
There was a problem hiding this comment.
Is your smoke test just an instance of e2e tests? Maybe saying 'not smoke' and 'not e2e' is redundant? I'm not sure.
ofrancon
left a comment
ofrancon
left a comment
There was a problem hiding this comment.
Looks reasonable to me but please put the requirements either in requirements-build.txt or in the e2e folder, but not both - that's confusing. Make sure the doc reflects your choice. Thanks!
| pexpect | ||
| pyhocon | ||
| pytest-xdist | ||
| pytest-timeout |
There was a problem hiding this comment.
Should these requirements go to tests/e2e/requirements.txt then?
4 participants
This PR is set to have all the general infrastructure to create e2e tests like [smoke test, load test, etc.)
Note: This e2e test does not use any of Dan's test infrastructure for this PR. It would be the next to do.
This directory contains the complete end-to-end (E2E) test infrastructure for the
music_nerd_proagent, including configuration, reusable utilities, test cases, and server lifecycle control tools.I significantly refactored my first version, which had a start/stop server as its tool files. I also made a base e2e test case that can be called by pytest manually, which allowed me to create our smoke test as below.
📁 Directory Structure