Skip to content

Fix unused Playwright test argument lint#162

Merged
shayancoin merged 1 commit intomainfrom
codex/update-synthetic-performance-tests-syntax
Oct 18, 2025
Merged

Fix unused Playwright test argument lint#162
shayancoin merged 1 commit intomainfrom
codex/update-synthetic-performance-tests-syntax

Conversation

@shayancoin
Copy link
Owner

@shayancoin shayancoin commented Oct 17, 2025

Summary

  • rename the unused Playwright test fixture parameter to avoid lint issues while keeping testInfo accessible

Testing

  • not run

https://chatgpt.com/codex/tasks/task_e_68f1af392ef483308cc0dfe48f6824af

Summary by CodeRabbit

  • Tests
    • Internal test infrastructure update with no impact on end-user functionality.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

A test callback parameter in tests/perf.p90.spec.ts is renamed from an unnamed empty object to an explicitly named _unused variable, clarifying intent without altering runtime behavior or test logic.

Changes

Cohort / File(s) Change Summary
Test callback parameter refactoring
tests/perf.p90.spec.ts
Renamed first parameter from empty object {} to _unused, maintaining function signature and testInfo parameter binding

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A parameter once left unnamed,
Now _unused it proudly claimed,
No logic bent, no test reframed—
Just clarity, elegantly framed! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not follow the required template structure specified in the repository. The required "PR Type" section is completely missing, and while the description provides a "Summary" section and "Testing" section, these do not match the required "Short Description" and "Tests Added" sections from the template. Although the description contains substantive information about the change, it fails to adhere to the mandated template format. Update the pull request description to follow the required template structure. Add a "PR Type" section and designate this as "Fix" since it addresses a lint issue. Rename "Summary" to "Short Description" and "Testing" to "Tests Added" to match the template requirements, ensuring all required sections are properly labeled.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Fix unused Playwright test argument lint" directly and clearly summarizes the main change in the pull request. It accurately reflects the core objective of renaming an unused test parameter to resolve a lint issue in the test file, which matches exactly what the changeset accomplishes according to the summary.
✨ Finishing touches
  • 📝 Docstrings were successfully generated.
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/update-synthetic-performance-tests-syntax

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9bde323 and 219a5b7.

📒 Files selected for processing (1)
  • tests/perf.p90.spec.ts (1 hunks)
🧰 Additional context used
🔍 Remote MCP

Let me search for documentation about Playwright testing best practices and the lint rule being addressed.Based on my research, I have gathered relevant context for reviewing this pull request. Let me provide a summary:

Summary of Findings

Context about the Fix

The issue being addressed is that Playwright's test function requires the first parameter to use object destructuring, which means using an empty object {} instead of a simple underscore _ for unused parameters, as TypeScript developers normally do.

The standard ESLint approach to handle unused parameters is to use an ignore pattern with a leading underscore (e.g., _unused), where ESLint is configured to ignore parameters starting with an underscore through the argsIgnorePattern rule.

Playwright Test Parameter Conventions

Playwright recommends TypeScript and linting with ESLint for tests, particularly using the @typescript-eslint/no-floating-promises rule to catch missing awaits on asynchronous calls. The naming convention change from {} to _unused aligns with broader ESLint best practices for marking unused variables.

Why This Change Matters

The change converts the test signature from:

  • test('name', async ({}, testInfo) => { ... })test('name', async (_unused, testInfo) => { ... })

This addresses a lint warning while maintaining the same functionality since:

  1. The first parameter (built-in Playwright fixtures) is genuinely unused in this specific test
  2. testInfo remains accessible as the second parameter
  3. Runtime behavior is completely unchanged

[::web_search::]

🔇 Additional comments (1)
tests/perf.p90.spec.ts (1)

155-155: LGTM! Clean fix for the lint warning.

The rename from {} to _unused correctly addresses the unused parameter lint issue while maintaining testInfo accessibility. This follows the standard ESLint convention for marking unused parameters with a leading underscore.

Based on previous findings.


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

@shayancoin shayancoin merged commit 3ce5e24 into main Oct 18, 2025
4 of 9 checks passed
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 18, 2025

Caution

Docstrings generation - FAILED

No docstrings were generated.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant