Fix review findings: crashes, dead code, add unit tests and lint#9
Merged
Conversation
- Initialize screenshot_path before the page branches in pytest_runtest_makereport so page-less or errored captures no longer crash the hook at context-storage time - tests/conftest.py login_page fixture called LoginPage.load_login_direct(), which does not exist; use navigate() - Replace dead report.config/report.markers logic in pytest_runtest_logreport: TestReport carries neither attribute, so max_reruns was always 0 and tags always []. Capture the pytest config in pytest_configure and derive tags from registered markers via report.keywords - Fix copy-paste docstrings (Hudl reference, nonexistent utils.ai_healing_decorator_fixed module)
LoginPage and SecurePage hardcoded the-internet.herokuapp.com, so the documented BASE_URL env config never reached navigation. Build page URLs from settings.BASE_URL instead.
- Delete conftest.py.backup / conftest.py.original from repo root - Delete utils/context_capture.py (unused, sync-API module in an async framework) and utils/decorators/retry_decorator.py (never imported, duplicates pytest-rerunfailures) - Remove committed Allure reports/results/screenshots (~7 MB; the reason GitHub labeled this repo as HTML) and ignore test_artifacts/allure/ - Stop ignoring visual_baselines/ - baselines are intentionally tracked - Drop pydantic from requirements (never imported); add ollama, which the framework imports but the requirements file omitted - Drop AI_HEALING_ENDPOINT from .env.example (nothing reads it)
Cover the pure logic that previously had zero tests: - test_observability: categorize_error categories/precedence, summarize aggregation (pass/flake rates, category grouping, slowest ordering), record enable-gating and JSONL output - jira_client: extract_ticket_id patterns and boundary quirks, format_result_comment status/emoji/truncation/noformat rendering - visual_regression: compare_images identity, full/partial diffs, tolerance, size-mismatch resize, missing-file handling - ai_healing: all fallback strategies of _parse_ollama_response against malformed LLM output, plus strip_style_tags These run without a browser or Ollama service.
- Replace bare except clauses with except Exception (ai_healing) - Remove f-string prefixes with no placeholders - Remove unused pathlib.Path import and dead page_source tracking in screenshot_decorator
…eanup - Remove the unpinned 'curl ... | sh' Ollama install from all three jobs; AI healing is disabled in CI and the Python client now comes from requirements, so this was pure supply-chain exposure - Remove AI_HEALING_ENDPOINT from CI env (nothing reads it) - Add a ruff lint job and a browser-free unit-test job - README: fix mangled Windows activation command (contained a literal BEL from an unescaped \a), close unclosed code fence, remove duplicate Directory Structure section, sync tree with reality, fix typos - pytest.ini: fix 'somethign' typo
ollama requires httpx>=0.27; the old 0.24.1 pin only worked because CI installed ollama in a separate pip step after requirements. httpx is not imported directly anywhere in the framework.
|
✅ All smoke tests passed! View report |
There was a problem hiding this comment.
Pull request overview
This PR addresses previously identified stability and correctness issues in the framework (AI healing hook, pytest observability metadata, and page-object configuration), while also improving maintainability via cleanup, adding browser-free unit tests for core utility logic, and tightening CI hygiene.
Changes:
- Fixes multiple runtime issues in pytest hooks/fixtures (AI-healing crash path, login fixture, and observability rerun/tag derivation).
- Wires
settings.BASE_URLthrough key page objects and removes unused/dead code and artifacts. - Adds a new
tests/unit/suite plus CI jobs forrufflinting and unit-only execution.
Reviewed changes
Copilot reviewed 23 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/test_observability.py | Removes an unused import; supports updated observability behavior. |
| utils/network_mocking.py | Minor print-string cleanup (removes unnecessary f-strings). |
| utils/decorators/screenshot_decorator.py | Removes unused page_source tracking and a commented debug print. |
| utils/decorators/retry_decorator.py | Deletes unused retry decorator module. |
| utils/context_capture.py | Deletes unused context-capture module. |
| utils/ai_healing.py | Tightens exception handling and minor console-output cleanup. |
| tests/visual_regression/test_visual_regression.py | Minor print-string cleanup (removes unnecessary f-strings). |
| tests/unit/test_visual_regression.py | Adds unit tests for compare_images() behavior. |
| tests/unit/test_test_observability.py | Adds unit tests for error categorization, record gating, and summary aggregation. |
| tests/unit/test_jira_client.py | Adds unit tests for ticket extraction and Jira comment formatting. |
| tests/unit/test_ai_healing_parser.py | Adds unit tests covering _parse_ollama_response fallbacks and strip_style_tags. |
| tests/unit/init.py | Adds unit-test package marker. |
| tests/conftest.py | Fixes login fixture to use navigate() and updates suite description text. |
| requirements_with_versions.txt | Removes unused pydantic; adds missing ollama dependency; deduplicates visual deps comments. |
| README.md | Fixes Windows venv activation command, updates Ollama install notes, and syncs directory structure/docs. |
| pytest.ini | Fixes a typo in marker documentation. |
| pages/secure_page.py | Uses settings.BASE_URL for secure page URL. |
| pages/login_page.py | Uses settings.BASE_URL for login page URL. |
| pages/base_page.py | Updates docstring example (now references BASE_URL-driven URLs). |
| conftest.py.original | Deletes obsolete backup conftest. |
| conftest.py.backup | Deletes obsolete backup conftest. |
| conftest.py | Fixes AI-healing hook crash path, captures pytest config for rerun/tag logic, and formats code for linting. |
| .gitignore | Gitignores Allure artifacts directory and updates visual artifact ignore rules/comments. |
| .github/workflows/smoke_full.yml | Adds lint and unit jobs; removes Ollama CLI install and unused env var. |
| .env.example | Removes unused AI_HEALING_ENDPOINT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
47
to
50
| def __init__(self, page): | ||
| super().__init__(page) | ||
| self.url = "https://the-internet.herokuapp.com/login" | ||
| self.url = f"{settings.BASE_URL}/login" | ||
|
|
Comment on lines
46
to
49
| def __init__(self, page): | ||
| super().__init__(page) | ||
| self.url = "https://the-internet.herokuapp.com/secure" | ||
| self.url = f"{settings.BASE_URL}/secure" | ||
|
|
Comment on lines
18
to
+24
| from pages.base_page import BasePage | ||
|
|
||
| class LoginPage(BasePage): | ||
| def __init__(self, page): | ||
| super().__init__(page) | ||
| self.url = "https://the-internet.herokuapp.com/login" | ||
| self.url = f"{settings.BASE_URL}/login" | ||
|
|
| ``` | ||
|
|
||
| You can see some example reports and attempts at fixing the files in the example_ai_reports folder | ||
| You can see some example reports and attempts at fixing the files in `test_artifacts/ai/ai_healing_reports/` |
|
✅ All chromium login tests passed! View report |
|
✅ All firefox login tests passed! View report |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the high/medium-priority findings from the repo review.
Bug fixes
UnboundLocalErrorin the AI-healing hook —screenshot_pathwas only assigned in the happy path ofpytest_runtest_makereport; any failure without a recognized page fixture (or an exception incapture_failure_context) crashed the hook. Now initialized toNoneup front.login_pagefixture — calledLoginPage.load_login_direct(), which doesn't exist; now usesnavigate().report.config/report.markerslogic — pytestTestReporthas neither attribute, so observabilitymax_rerunswas always 0 andtagsalways[]. The pytest config is now captured inpytest_configure, and tags are derived from registered markers intersected withreport.keywords.BASE_URLwired through page objects —LoginPage/SecurePagehardcoded the-internet.herokuapp.com; they now build URLs fromsettings.BASE_URL, making the documented env config real.New unit tests (
tests/unit/, 50+ cases, no browser/Ollama needed)test_observability:categorize_errorcategories and precedence,summarize()aggregation (pass/flake rates, failure grouping, slowest ordering),record()gating and JSONL outputjira_client:extract_ticket_idpatterns (including the underscore/word-boundary quirk),format_result_commentrendering and truncationvisual_regression:compare_imagesidentity/partial/full diffs, tolerance, size-mismatch resize, missing filesai_healing: every fallback strategy of_parse_ollama_responseagainst malformed LLM output, plusstrip_style_tagsRepo hygiene
conftest.py.backup,conftest.py.original,utils/context_capture.py(unused),utils/decorators/retry_decorator.py(never imported)test_artifacts/allure/;visual_baselines/remains trackedpydanticfrom requirements (never imported); addedollama(imported but missing)AI_HEALING_ENDPOINTfrom.env.exampleand CI (nothing reads it)CI
curl -fsSL https://ollama.com/install.sh | shfrom all three jobs — Ollama is disabled in CI, so this was pure supply-chain exposurerufflint job (codebase is now lint-clean) and a browser-freeunitjob runningtests/unitDocs
.venv\Scriptsctivate— the file contained a literal BEL byte), closed an unclosed code fence, removed the duplicate Directory Structure section and synced the tree with reality, fixed typos in README andpytest.iniVerification
ruff check .passes locally; all modules compileunitCI job on this PR