Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
*.log
*.png
__pycache__/
53 changes: 53 additions & 0 deletions tests/test_portfolio.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from playwright.sync_api import sync_playwright
import time

def run(playwright):
Comment on lines +1 to +4
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Structure this as a real test function (e.g. pytest) rather than a standalone script

As written, this is a manual verification script: it runs on import and uses prints instead of assertions. Please convert it into one or more test_... functions (e.g. def test_portfolio_hud_visible(page): ...) using Playwright’s pytest integration/fixtures so it’s executed by the test runner and produces proper pass/fail results in CI.

browser = playwright.chromium.launch(headless=True)
page = browser.new_page()

# Capture console messages
page.on("console", lambda msg: print(f"CONSOLE: {msg.text}"))
page.on("pageerror", lambda err: print(f"PAGE ERROR: {err}"))
Comment on lines +4 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Fail the test on console errors or page errors instead of just printing them

Right now these handlers only log issues, so the test can pass despite serious runtime problems. Instead, collect console/page messages and assert that none are of type "error", or raise inside the handlers so that any JavaScript error causes the test to fail.

Suggested change
def run(playwright):
browser = playwright.chromium.launch(headless=True)
page = browser.new_page()
# Capture console messages
page.on("console", lambda msg: print(f"CONSOLE: {msg.text}"))
page.on("pageerror", lambda err: print(f"PAGE ERROR: {err}"))
def run(playwright):
def _handle_console_message(msg):
# Fail the test immediately on any console error
if getattr(msg, "type", None) == "error":
raise AssertionError(f"Console error: {msg.text}")
print(f"CONSOLE: {msg.text}")
def _handle_page_error(err):
# Any page error should also fail the test
raise AssertionError(f"PAGE ERROR: {err}")
browser = playwright.chromium.launch(headless=True)
page = browser.new_page()
# Capture console messages and fail on errors
page.on("console", _handle_console_message)
page.on("pageerror", _handle_page_error)


try:
print("Navigating to http://localhost:8000/index.html")
page.goto("http://localhost:8000/index.html")

# Wait for potential initial animations or loading
time.sleep(2)

print("Taking initial screenshot...")
Comment on lines +13 to +19
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Avoid fixed sleep for page readiness; wait on concrete conditions instead

Using time.sleep(2) makes this test both slow and flaky across environments. Prefer Playwright waits (for example, page.wait_for_load_state("networkidle") or page.wait_for_selector(".hud", timeout=...)) so the test advances as soon as the page is ready and fails explicitly if the expected state is never reached.

Suggested change
print("Navigating to http://localhost:8000/index.html")
page.goto("http://localhost:8000/index.html")
# Wait for potential initial animations or loading
time.sleep(2)
print("Taking initial screenshot...")
print("Navigating to http://localhost:8000/index.html")
page.goto("http://localhost:8000/index.html")
# Wait explicitly for the page to be ready and the HUD to appear
page.wait_for_load_state("networkidle")
page.wait_for_selector(".hud", timeout=10000)
print("Taking initial screenshot...")

page.screenshot(path="tests/initial.png")
Comment on lines +19 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Clarify how screenshots are used (assertions or baselines) and consider naming/output structure

Right now the screenshots are only written to disk and never asserted. If they’re intended for visual regression, consider comparing them to a baseline or at least publishing them as CI artifacts. It may also help to write them under a dedicated directory (e.g. tests/screenshots/ or tests/artifacts/) so they’re easy to organize and .gitignore.

Suggested implementation:

import os
import time
        # Wait for potential initial animations or loading
        time.sleep(2)

        # Take an initial screenshot for debugging / visual inspection (e.g. CI artifacts)
        screenshot_dir = os.path.join("tests", "screenshots")
        os.makedirs(screenshot_dir, exist_ok=True)

        print("Taking initial screenshot...")
        page.screenshot(path=os.path.join(screenshot_dir, "initial.png"))

To fully implement the suggestion about CI artifacts and/or baselines, you will also want to:

  1. Configure your CI pipeline (e.g. GitHub Actions, GitLab CI) to upload tests/screenshots/ as an artifact on test runs.
  2. If you later add visual regression, introduce a baseline directory (e.g. tests/screenshots/baseline/) and comparison logic, and update this test to either:
    • Compare the new screenshot with a stored baseline, or
    • Generate/update the baseline in a dedicated "approve" flow rather than in normal test runs.


# Check for key elements (the HUD)
if page.locator(".hud").is_visible():
Comment on lines +22 to +23
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Use assertions for HUD visibility instead of printing, and handle missing locator robustly

This only logs HUD visibility and doesn’t affect the test result. Instead, assert that .hud exists and is visible (e.g. expect(page.locator('.hud')).to_be_visible() or equivalent assertions). Also account for the case where .hud is missing: is_visible() on a non-existent element can be misleading, so explicitly wait for the HUD selector with a timeout to produce clearer failures.

Suggested implementation:

        print("Taking initial screenshot...")
        page.screenshot(path="tests/initial.png")

        # Assert key elements (the HUD) are present and visible
        hud = page.locator(".hud")
        expect(hud).to_be_visible(timeout=5000)

To fully support this change, ensure that expect is imported from Playwright at the top of tests/test_portfolio.py, for example:

from playwright.sync_api import expect

If the file already imports from playwright.sync_api, you can extend that import to include expect rather than adding a new import line.

print("HUD is visible.")
else:
print("HUD is NOT visible.")

# Simulate mouse movement
print("Simulating mouse movement...")
page.mouse.move(100, 100)
time.sleep(0.5)
page.mouse.move(500, 500)
time.sleep(0.5)

# Simulate click (Warp)
print("Simulating click (Warp)...")
page.mouse.down()
Comment on lines +28 to +37
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add assertions around the mouse interactions to verify the "warp" effect, not just exercise it

These interactions are only executed, not validated. To ensure the warp effect and HUD behavior are actually tested, add assertions on observable outcomes (e.g., DOM/CSS changes, element visibility, or a pixel/screenshot comparison) rather than just confirming no exceptions are thrown.

Suggested implementation:

        print("Taking initial screenshot...")
        page.screenshot(path="tests/initial.png")

        hud = page.locator(".hud")
        assert hud.count() > 0, "Expected HUD element '.hud' to exist on the page"

        initial_hud_box = hud.bounding_box()

        # Check for key elements (the HUD)
        if hud.is_visible():
            print("HUD is visible.")
        else:
            print("HUD is NOT visible.")
        assert hud.is_visible(), "HUD should be visible before warp interaction"

        # Simulate mouse movement intended to trigger the warp effect
        print("Simulating mouse movement...")
        page.mouse.move(100, 100)
        page.wait_for_timeout(500)
        page.mouse.move(500, 500)
        page.wait_for_timeout(500)

        # Simulate click (Warp)
        print("Simulating click (Warp)...")
        page.mouse.down()
        page.wait_for_timeout(1000)  # Hold for a bit to let the warp effect happen

        print("Taking warping screenshot...")
        page.screenshot(path="tests/warping.png")

        page.mouse.up()
        page.wait_for_timeout(1000)

        # Re-query HUD to avoid stale references and validate warp effect
        hud_after_warp = page.locator(".hud")
        assert hud_after_warp.is_visible(), "HUD should remain visible after warp interaction"

        warped_hud_box = hud_after_warp.bounding_box()
        assert (
            initial_hud_box is not None and warped_hud_box is not None
        ), "HUD bounding boxes must be measurable"

        # Assert that the HUD has visually changed position or size as a proxy for the warp effect
        hud_position_changed = (
            initial_hud_box["x"] != warped_hud_box["x"]
            or initial_hud_box["y"] != warped_hud_box["y"]
            or initial_hud_box["width"] != warped_hud_box["width"]
            or initial_hud_box["height"] != warped_hud_box["height"]
        )
        assert hud_position_changed, "Expected HUD position or size to change after warp interaction"

If the warp effect is represented by a specific DOM/CSS change (e.g., a class like .warp-active or a CSS transform on a particular element), you can strengthen the assertions by checking for that explicit signal instead of (or in addition to) bounding box changes. For example, assert on page.locator(".hud").evaluate("el => getComputedStyle(el).transform") or the presence of a warp-specific class once you align the selector with the actual implementation.

time.sleep(1) # Hold for a bit to let the warp effect happen

print("Taking warping screenshot...")
page.screenshot(path="tests/warping.png")

page.mouse.up()
time.sleep(1)

except Exception as e:
print(f"ERROR: {e}")
Comment on lines +46 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Avoid swallowing exceptions so the test fails when something goes wrong

Catching Exception and only printing it will hide test failures and make the test appear to pass. Either re-raise after logging or allow the exception to propagate so the test fails correctly. If you need cleanup, use a fixture or try/finally without a broad except.

Comment on lines +8 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "test_portfolio.py" -type f

Repository: AshrafMorningstar/3d-portfolio

Length of output: 99


🏁 Script executed:

cat -n ./tests/test_portfolio.py | head -60

Repository: AshrafMorningstar/3d-portfolio

Length of output: 1932


The test never fails because errors are only printed and exceptions are caught without being re-raised.

Console/page errors, HUD invisibility, and exceptions are currently logged but do not cause test failure. The exception handler (lines 46–47) catches all errors and prints them without re-raising, causing the test to pass silently even when failures occur.

🐛 Proposed fix to assert failures
-    # Capture console messages
-    page.on("console", lambda msg: print(f"CONSOLE: {msg.text}"))
-    page.on("pageerror", lambda err: print(f"PAGE ERROR: {err}"))
+    console_errors = []
+    page_errors = []
+    # Capture console messages
+    def on_console(msg):
+        if msg.type == "error":
+            console_errors.append(msg.text)
+        print(f"CONSOLE: {msg.text}")
+    def on_page_error(err):
+        page_errors.append(str(err))
+        print(f"PAGE ERROR: {err}")
+    page.on("console", on_console)
+    page.on("pageerror", on_page_error)
@@
-        if page.locator(".hud").is_visible():
-            print("HUD is visible.")
-        else:
-            print("HUD is NOT visible.")
+        if not page.locator(".hud").is_visible():
+            raise AssertionError("HUD is NOT visible.")
+        print("HUD is visible.")
@@
-    except Exception as e:
-        print(f"ERROR: {e}")
+        if console_errors or page_errors:
+            raise AssertionError(
+                f"Console/page errors detected: {console_errors + page_errors}"
+            )
+    except Exception as e:
+        print(f"ERROR: {e}")
+        raise
🧰 Tools
🪛 Ruff (0.14.14)

46-46: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
In `@tests/test_portfolio.py` around lines 8 - 47, The test currently swallows
errors by catching all exceptions and only printing them, and it doesn’t assert
HUD visibility or fail on console/page errors; update the test to fail on any
errors by (1) attaching page.on("console", ...) and page.on("pageerror", ...)
handlers that append messages to a list and assert that list is empty at the
end, (2) replace the broad try/except around interactions (or re-raise the
exception after logging) so exceptions propagate to the test runner, and (3) add
an explicit assertion that page.locator(".hud").is_visible() is True (or raise
an AssertionError) to ensure HUD invisibility causes test failure; reference the
existing handlers (page.on), navigation (page.goto), HUD check
(page.locator(".hud").is_visible()), and the try/except block to locate changes.


finally:
browser.close()

with sync_playwright() as playwright:
run(playwright)
Comment on lines +52 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

wc -l tests/test_portfolio.py

Repository: AshrafMorningstar/3d-portfolio

Length of output: 100


🏁 Script executed:

cat -n tests/test_portfolio.py

Repository: AshrafMorningstar/3d-portfolio

Length of output: 1932


Wrap Playwright execution in a test function.
Lines 52–53 execute at module import time, which causes pytest to launch the browser and take screenshots during test collection rather than during test execution. This introduces unintended side effects and makes the file unsuitable as a pytest test module.

✅ Proposed fix
+def test_portfolio():
+    with sync_playwright() as playwright:
+        run(playwright)
-
-with sync_playwright() as playwright:
-    run(playwright)

Alternatively, if this file is intended as a standalone script, use an if __name__ == "__main__": guard instead of wrapping in a test function.

🤖 Prompt for AI Agents
In `@tests/test_portfolio.py` around lines 52 - 53, The Playwright call runs at
import time; wrap the execution so pytest won't run it during collection: move
the with sync_playwright() as playwright: run(playwright) block into a test
function (e.g., def test_portfolio_playwright(): ... ) so Playwright is started
during test execution, or if this is a standalone script, put that block under
an if __name__ == "__main__": guard; update references to sync_playwright and
run(playwright) accordingly.