-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[webview_flutter] Use a local web server for legacy web integration tests #5956
[webview_flutter] Use a local web server for legacy web integration tests #5956
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be much more stable! (Small question on using the loopback instead of "any" when binding the server)
const String secondaryUrl = 'https://www.google.com/robots.txt'; | ||
const String primaryPage = 'first.txt'; | ||
const String secondaryPage = 'second.txt'; | ||
final HttpServer server = await HttpServer.bind(InternetAddress.anyIPv4, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be:
final HttpServer server = await HttpServer.bind(InternetAddress.anyIPv4, 0); | |
final HttpServer server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); |
So you can connect only from http://localhost:${server.port}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work; done. The previous version was just copying from other tests (I think Ian wrote the first one.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the other version just opens a random port on the machine that could potentially be connected to from the outside world? (I don't know the security ramifications, but if I do that internally at work I get an automated email flagging it :P)
flutter/packages@841fe90...8fbdf65 2024-01-23 stuartmorgan@google.com [webview_flutter] Use a local web server for legacy web integration tests (flutter/packages#5956) 2024-01-23 engine-flutter-autoroll@skia.org Roll Flutter from 3ee8ff2 to 5b673c2 (28 revisions) (flutter/packages#5965) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…ests (flutter#5956) Use a local web server for the legacy test, just as for the non-legacy version. Also updates the repo tooling to ensure that this test file is found, so it's run regardless of whether tests are run by directory or by individual path. Part of flutter/flutter#95420
Use a local web server for the legacy test, just as for the non-legacy version.
Also updates the repo tooling to ensure that this test file is found, so it's run regardless of whether tests are run by directory or by individual path.
Part of flutter/flutter#95420
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).