Skip to content

fix: update StopDockerChrome calls to match new API signature#9

Merged
adnaan merged 2 commits intomainfrom
fix/stop-docker-chrome-api
Nov 30, 2025
Merged

fix: update StopDockerChrome calls to match new API signature#9
adnaan merged 2 commits intomainfrom
fix/stop-docker-chrome-api

Conversation

@adnaan
Copy link
Copy Markdown
Contributor

@adnaan adnaan commented Nov 30, 2025

Summary

  • Updated e2etest.StopDockerChrome calls in 3 test files to match the new function signature
  • The API changed from (t, chromeCmd, debugPort) to (t, debugPort)
  • Affected files: counter/counter_test.go, avatar-upload/avatar-upload_test.go, todos/todos_test.go

Test plan

  • Ran ./test-all.sh - all tests pass

🤖 Generated with Claude Code

The e2etest.StopDockerChrome function signature changed from
(t, chromeCmd, debugPort) to (t, debugPort). Updated all test
files to use the new signature.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings November 30, 2025 20:15
@adnaan
Copy link
Copy Markdown
Contributor Author

adnaan commented Nov 30, 2025

Closing: The actual issue is in the lvt module. The local go.work uses a newer unpublished API, but CI uses the published version with the old signature. The lvt module needs to be updated and published first.

@adnaan adnaan closed this Nov 30, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the e2etest.StopDockerChrome function calls in three test files to match the new API signature that no longer requires the chromeCmd parameter. The API was changed from (t, chromeCmd, debugPort) to (t, debugPort).

  • Updated function calls to use the new two-parameter signature (t, debugPort)
  • Retained assignment of chromeCmd return value but discarded it with blank identifier pattern
  • Added explanatory comments about the discarded return value

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
todos/todos_test.go Updated StopDockerChrome call to use new signature and added blank identifier assignment for unused chromeCmd
counter/counter_test.go Updated StopDockerChrome call to use new signature and added blank identifier assignment for unused chromeCmd
avatar-upload/avatar-upload_test.go Updated StopDockerChrome call to use new signature and added blank identifier assignment for unused chromeCmd

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 107 to +109
chromeCmd := e2etest.StartDockerChrome(t, debugPort)
defer e2etest.StopDockerChrome(t, chromeCmd, debugPort)
defer e2etest.StopDockerChrome(t, debugPort)
_ = chromeCmd // Command returned for reference; cleanup handled by StopDockerChrome
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The blank identifier assignment pattern is unnecessarily verbose. The variable chromeCmd is assigned on line 107, then used in line 109 just to discard it with _ = chromeCmd.

For consistency with login/login_test.go (which already uses the updated API), consider using the blank identifier directly in the assignment:

_ = e2etest.StartDockerChrome(t, debugPort)
defer e2etest.StopDockerChrome(t, debugPort)

This eliminates the need for the explanatory comment and is more idiomatic Go.

Copilot uses AI. Check for mistakes.
Comment on lines 55 to +57
chromeCmd := e2etest.StartDockerChrome(t, debugPort)
defer e2etest.StopDockerChrome(t, chromeCmd, debugPort)
defer e2etest.StopDockerChrome(t, debugPort)
_ = chromeCmd // Command returned for reference; cleanup handled by StopDockerChrome
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The blank identifier assignment pattern is unnecessarily verbose. The variable chromeCmd is assigned on line 55, then used in line 57 just to discard it with _ = chromeCmd.

For consistency with login/login_test.go (which already uses the updated API), consider using the blank identifier directly in the assignment:

_ = e2etest.StartDockerChrome(t, debugPort)
defer e2etest.StopDockerChrome(t, debugPort)

This eliminates the need for the explanatory comment and is more idiomatic Go.

Copilot uses AI. Check for mistakes.
Comment on lines 50 to +52
chromeCmd := e2etest.StartDockerChrome(t, debugPort)
defer e2etest.StopDockerChrome(t, chromeCmd, debugPort)
defer e2etest.StopDockerChrome(t, debugPort)
_ = chromeCmd // Command returned for reference; cleanup handled by StopDockerChrome
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The blank identifier assignment pattern is unnecessarily verbose. The variable chromeCmd is assigned on line 50, then used in line 52 just to discard it with _ = chromeCmd.

For consistency with login/login_test.go (which already uses the updated API), consider using the blank identifier directly in the assignment:

_ = e2etest.StartDockerChrome(t, debugPort)
defer e2etest.StopDockerChrome(t, debugPort)

This eliminates the need for the explanatory comment and is more idiomatic Go.

Copilot uses AI. Check for mistakes.
Updates github.com/livetemplate/lvt to v0.0.0-20251130141940-9b94cde94e9d
which includes the new StopDockerChrome API signature.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@adnaan adnaan reopened this Nov 30, 2025
@adnaan adnaan merged commit 9dd1895 into main Nov 30, 2025
9 checks passed
@adnaan adnaan deleted the fix/stop-docker-chrome-api branch November 30, 2025 20:23
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.

2 participants