Skip to content

test: move poll_deployment_health tests out of e2e#849

Merged
muddlebee merged 2 commits intoTracer-Cloud:mainfrom
imjohnzakkam:issue/840-move-deploy-health-tests
Apr 28, 2026
Merged

test: move poll_deployment_health tests out of e2e#849
muddlebee merged 2 commits intoTracer-Cloud:mainfrom
imjohnzakkam:issue/840-move-deploy-health-tests

Conversation

@imjohnzakkam
Copy link
Copy Markdown
Contributor

@imjohnzakkam imjohnzakkam commented Apr 24, 2026

Fixes #840

Describe the changes you have made in this PR -

Moved the pure mocked poll_deployment_health tests out of tests/e2e/deploy/ and into tests/test_deployment_health.py.

This keeps the mocked test behavior unchanged while making the tests discoverable in the default pytest run.

Screenshots of the UI changes (If any) -

  • None

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:
The issue calls out tests/e2e/deploy/test_deploy_timing.py as a pure mocked test module that should be collected by the default suite. I moved it to tests/test_deployment_health.py without changing the assertions or retry behavior.

One edge case I considered was the destination path. pytest.ini excludes both tests/e2e and tests/deployment from normal recursion, so moving these tests under tests/deployment/ would still keep them out of the default run. I placed the file directly under tests/ so it remains easy to discover and is collected by default.

Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • My code follows the project's style guidelines and conventions

Verification

  • python -m pytest tests/test_deployment_health.py
  • python -m pytest --collect-only tests/test_deployment_health.py

Output summary:

  • 3 passed
  • 3 tests collected

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026 •
edited
Loading

Greptile Summary

This PR moves three purely mocked poll_deployment_health tests from tests/e2e/deploy/test_deploy_timing.py to tests/test_deployment_health.py, where they are discovered by the default pytest run (both tests/e2e and tests/deployment are excluded in pytest.ini's norecursedirs). A module docstring is added; logic is otherwise unchanged.

Confidence Score: 5/5

Safe to merge — this is a pure test relocation with no production code changes.

All three test assertions are verified correct against the implementation. The only finding is a P2 style suggestion to use pytest.raises instead of a manual try/except block, which does not affect correctness or coverage.

No files require special attention.

Important Files Changed

Filename Overview
tests/test_deployment_health.py Pure rename from tests/e2e/deploy/test_deploy_timing.py with a module docstring added; all three test assertions verified correct against the implementation in app/deployment/health.py.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["pytest default run"] -->|"norecursedirs excludes tests/e2e"| B["tests/e2e/deploy/test_deploy_timing.py\n(NOT collected before)"]
    A -->|"tests/ root is collected"| C["tests/test_deployment_health.py\n(collected now ✓)"]
    C --> D["test_poll_deployment_health_retries_until_success"]
    C --> E["test_poll_deployment_health_times_out_for_unreachable_endpoint"]
    C --> F["test_poll_deployment_health_uses_explicit_health_url_without_fallback"]
    D & E & F --> G["app/deployment/health.py :: poll_deployment_health"]
Loading

Comments Outside Diff (1)

  1. tests/test_deployment_health.py, line 49-62 (link)

    P2 Prefer pytest.raises over manual try/except

    Using a bare try/except/else: raise AssertionError pattern works but is harder to read and easy to get wrong (e.g. if the except block itself raises, the else branch never runs). pytest.raises is idiomatic, shorter, and produces cleaner failure messages.

    You'll also need import pytest at the top of the file.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "test: move poll_deployment_health tests ..." | Re-trigger Greptile

@imjohnzakkam
Copy link
Copy Markdown
Contributor Author

imjohnzakkam commented Apr 24, 2026

Updated. Switched that case to pytest.raises, added the pytest import, and re-ran the focused test file successfully.

@muddlebee muddlebee merged commit f39b3db into Tracer-Cloud:main Apr 28, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Move the pure poll_deployment_health tests out of tests/e2e/

2 participants

@imjohnzakkam @muddlebee