Skip to content

Add package:http requests to network integration test#9016

Merged
srawlins merged 3 commits intoflutter:masterfrom
srawlins:cupertino
Mar 12, 2025
Merged

Add package:http requests to network integration test#9016
srawlins merged 3 commits intoflutter:masterfrom
srawlins:cupertino

Conversation

@srawlins
Copy link
Copy Markdown
Contributor

Work towards #7554.

@srawlins srawlins requested a review from a team as a code owner March 10, 2025 22:31
@srawlins srawlins requested review from kenzieschmoll and removed request for a team March 10, 2025 22:31
}

// TODO WebSocket
// TODO package:http - BrowserClient - https://pub.dev/documentation/http/latest/browser_client/BrowserClient-class.html
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I forgot that Flutter DevTools is not geared towards Flutter Web apps, where users will more likely use their browser's developer tools in order to examine network requests. So 'package:http - BrowserClient' and fetch_client are out.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Correct, only some of the tools are available for web apps. Any screen that has requiresDartVm: true is not enabled for web: https://github.com/flutter/devtools/blob/master/packages/devtools_app/lib/src/shared/framework/screen.dart/#L80

await helper.clear();

// Instruct the app to make a POST request via the 'http' package.
await helper.triggerRequest('packageHttp/post/');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need a mirrored change in the test app to support this api?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No they were already there (as dead code); I missed them.


// Instruct the app to make a GET request via the 'http' package.
await helper.triggerRequest('packageHttp/get/');
_expectInRequestTable('GET');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this expect statement could be true because of the get request on line 41. We need to ensure we can uniquely identify the requests in the table, either with some key or by checking findsNWidgets instead which should increase for each request of the same method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We press the 'Clear' button between test cases.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah okay. Just to be certain, can we add an expect statement at the end of the helper.clear() method that verifies screenControllers.lookup().requests.value is empty?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, excellent, done!

Copy link
Copy Markdown
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

LGTM with one request

@srawlins srawlins merged commit cfff32d into flutter:master Mar 12, 2025
41 checks passed
@srawlins srawlins deleted the cupertino branch March 12, 2025 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants