Skip to content

test: increase timeout duration to 30 seconds for restaurant finder e…#1370

Open
sugoi-yuzuru wants to merge 1 commit intomainfrom
fixci
Open

test: increase timeout duration to 30 seconds for restaurant finder e…#1370
sugoi-yuzuru wants to merge 1 commit intomainfrom
fixci

Conversation

@sugoi-yuzuru
Copy link
Copy Markdown
Collaborator

…2e test probes

Description

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. For larger changes, raising an issue first helps reduce redundant work.

Pre-launch Checklist

If you need help, consider asking for advice on the discussion board.

Copy link
Copy Markdown
Contributor

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

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 adds a 30-second timeout to ShellProbe instances in the restaurant finder e2e tests. Feedback includes a request to complete the mandatory pre-review checklist in the PR description per the style guide and a suggestion to extract the hardcoded timeout duration into a single constant to improve maintainability.

expect(response, contains('A2UI'));
print('\nReceived agent card:\n$response\n');
},
timeout: const Duration(seconds: 30),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The pull request description is incomplete and the pre-launch checklist has not been filled out. According to the Repository Style Guide (Rule 18), PR descriptions should include the Pre-Review Checklist with all steps completed. Please update the description to provide context for the changes and confirm that all necessary steps (such as signing the CLA and verifying tests) have been taken.

References
  1. PR descriptions should include the Pre-Review Checklist from the PR template, with all of the steps completed. (link)

expect(response, contains('"parts":[{"kind":'));
print('\nReceived agent response:\n$response\n');
},
timeout: const Duration(seconds: 30),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The timeout duration const Duration(seconds: 30) is hardcoded and duplicated here and on line 70. To improve maintainability and follow the existing pattern in this file (where URLs and commands are defined as constants), consider defining a single constant for the probe timeout at the top of the file.

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

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant