Skip to content

test(testnet): follow same-origin NFT links in the current tab - #275

Merged
JOY (JOY) merged 2 commits into
mainfrom
codex/testnet-nft-navigation-test
Oct 25, 2026
Merged

JOY (JOY) merged 2 commits into
mainfrom
codex/testnet-nft-navigation-test

Conversation

@JOY

@JOY JOY (JOY) commented Oct 25, 2026 •

Copy link
Copy Markdown

Summary

  • Correct the Testnet browser acceptance test to follow the live anchor target: same-origin NFT links have no target and navigate the existing page; only _blank links require a popup.
  • Only close a child tab, never the test parent page.
  • Add a text guard for both navigation branches. It failed against the old popup-only spec and passes with the correction.

Verification

  • Independent full delta review of f537c03..1ac2650: CLEAN_PASS, no P0-P3 findings.
  • node --check and git diff --check pass; affected guard red-to-green.
  • Actual Testnet browser activation from the DOS Name domain link navigated the same tab to the canonical ERC-1155 instance, showing its full ID and owner. The href is public HTTPS; target is null.
  • Runtime revision f537c03 is already deployed successfully (run 40200373407), with exact Compose/code hashes and healthy adapter verified on VM. This PR changes tests only; neither path matches Deploy Config push triggers, so no runtime redeployment is required.
  • Hosted browser CI was Cloudflare-skipped and is not counted as passing UI coverage. Real browser evidence supplies the navigation acceptance. Required final-head dependency CI remains a merge gate.
  • Mainnet remains untouched. This is not authenticated DOS.Me user claim/renew E2E evidence.

📌 TL;DR

This PR updates the Testnet UI E2E test to handle token links that navigate in the same tab (rather than always opening a popup), and adds a corresponding unit test to enforce this navigation logic in the test suite.

🎯 Type of Change

  • 🚀 New feature
  • 🐛 Bugfix
  • 🧹 Refactor
  • ⚡ Performance
  • 📚 Documentation
  • ⚙️ CI / Configuration

🔍 Changes Walkthrough

File Summary of Changes
.github/scripts/testnet-bens-ui.spec.mjs Modified the "DOS Name search" test to dynamically check the target attribute of the token link. If target="_blank", it waits for a popup; otherwise, it navigates in the current page. It also prevents closing the main page if navigation occurred in the same tab.
.github/scripts/tests/test_validate_testnet_bens.py Added a new test case test_browser_nft_guard_accepts_same_tab_and_popup_navigation that verifies the E2E spec file contains the expected logic for handling both same-tab and popup navigation scenarios.

📊 Architectural Flow

flowchart TD
    A[Start Test: DOS Name Search] --> B[Locate Token Link]
    B --> C{Check 'target' Attribute}
    C -->|"_blank"| D[Wait for Popup Event]
    C -->|Other| E[Click Link in Same Tab]
    D --> F[Set tokenPage to Popup]
    E --> G[Set tokenPage to Main Page]
    F --> H[Wait for DOM Content Loaded]
    G --> H
    H --> I[Assert URL & Content]
    I --> J{Is tokenPage != Main Page?}
    J -->|Yes| K[Close Popup]
    J -->|No| L[Skip Close]
    K --> M[End Test]
    L --> M
Loading

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.

Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-25T14:31:14.799830Z
We couldn’t complete this review. Comment @codex review to try again.
1ac2650 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c341d4a6-a0b0-4c7b-9e4e-0c074cba6428
Commits

Reviewing files that changed from the base of the PR and between f537c03 and f561667.

Files selected for processing: 2
  • .github/scripts/testnet-bens-ui.spec.mjs
  • .github/scripts/tests/test_validate_testnet_bens.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

dos[bot]
dos Bot approved these changes Oct 25, 2026

@dos dos Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⏱️ Code Review completed (2 eligible files · 2,491 chars · 1 model call(s) · 1 PR unit(s))

Whole diff with bounded context at base f537c03 / head 1ac2650.

✅ CLEAN_PASS: Complete diff reviewed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Playwright test suite to support both same-tab and popup navigation based on the token link's target attribute, and adds a Python unit test to verify these changes. Feedback suggests wrapping the waitForLoadState call in a conditional check to prevent potential race conditions during same-tab navigation, and using regular expressions in the Python unit test instead of exact string matching to make the test more robust against minor formatting changes.

Comment thread .github/scripts/testnet-bens-ui.spec.mjs Outdated
Comment thread .github/scripts/tests/test_validate_testnet_bens.py Outdated
dos[bot]
dos Bot approved these changes Oct 25, 2026

@dos dos Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⏱️ Code Review completed (2 eligible files · 2,663 chars · 1 model call(s) · 1 PR unit(s))

Whole diff with bounded context at base f537c03 / head f561667.

✅ CLEAN_PASS: Complete diff reviewed

@JOY

Copy link
Copy Markdown
Author

Final-head independent delta review of f561667 is CLEAN_PASS, no P0-P3 findings. Both native comments were fixed and resolved; affected source guard, node syntax and diff checks pass. Actual browser search-to-address and domain-to-NFT flows have now completed on the deployed runtime, with same-tab navigation confirmed. Required final-head CI remains the only merge gate. This PR is test-only and does not trigger a runtime deployment.

@JOY
JOY (JOY) merged commit 33f1e90 into main Oct 25, 2026
16 checks passed
@JOY

Copy link
Copy Markdown
Author

Completed: final-head dependency CI run 40203120252 passed, including 104 tests in 45.462s and real pinned adapter startup. Required review/conversation gates passed; merged as 33f1e90. Merged tree is identical to reviewed/tested head f561667. This test-only merge leaves the verified Testnet runtime at f537c03; Deploy Config was not triggered. Runtime/browser/contract acceptance and explicit limitations are recorded on PR274.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant