-
Notifications
You must be signed in to change notification settings - Fork 87
Wait until all scripts are parsed before recomputing metadata on a hot reload #2650
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
Conversation
dart-lang@2172ba7 added support to run tests in a temporary directory. This results in two flaky issues: 1. On Windows, build_daemon tests may fail to delete the temp directory because the process may have not been torn down yet, so it may still be accessing the file system. There was an initial retry after 1 second, but that appears to be not enough looking at a recent test run. 2. If a test times out, its tearDown may not be called. In this case, the ResidentWebRunner in frontend_server may not restore the current directory in the LocalFileSystem. This leads to cascading failures in subsequent tests due to no longer being in a path that contains 'webdev'. See https://github.com/dart-lang/webdev/actions/runs/15989286213/job/45099373212?pr=2641 for an example. See dart-lang/test#897 as well for tracking work to call tearDown on timeouts. To address the above issues: 1. Increase the delay between the two tries and assert this only occurs on Windows. 2. Cache the current directory so that it can be used in utilities.dart with the same (correct) value every time.
This reverts commit 3bd1448.
…t reload Fixes dart-lang#2640 On a hot reload, we wait until all the scripts are downloaded, but we don't wait until all of them are parsed and a script ID is created for them to refer to. This then results in incorrect metadata being computed. Instead, return the srcs that are being loaded during a hot reload, use a controller to determine when scripts are parsed, and only compute metadata once we have parsed all the reloaded scripts. In order to compare the parsed scripts' URLs with the reloaded scripts' URLs, we now require full URLs in the hot reload sources metadata. This is already true in Flutter tools, so this just canonicalizes that and modifies the tests to do the same. To be consistent, hot restart also provides the full URL in the DDC library bundle format and the bootstrap is modified to reflect that (and to clean up some unused code around module tracking).
We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Also cleans up some code that was never used around tracking modules in the bootstrap. Related PR: dart-lang/webdev#2650
|
Looks like there's some existing redness (see |
… a TODO to remove a test
We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
|
Failures are consistent with existing failures: https://github.com/dart-lang/webdev/actions/runs/16394079815 (both the specific tests and their failure logs) so I'm going to go ahead and land this. |
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…2271) We already use the baseUri when computing hot reload sources metadata as it can never be null. The member is changed to be non-nullable to reflect that. To be consistent, we also use the baseUri (full url) for a hot restart when running with the DDC library bundle format. Related PR: dart-lang/webdev#2650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
Fixes #2640
On a hot reload, we wait until all the scripts are downloaded, but we
don't wait until all of them are parsed and a script ID is created for
them to refer to. This then results in incorrect metadata being computed.
Instead, return the srcs that are being loaded during a hot reload,
use a controller to determine when scripts are parsed, and only
compute metadata once we have parsed all the reloaded scripts.
In order to compare the parsed scripts' URLs with the reloaded scripts'
URLs, we now require full URLs in the hot reload sources metadata.
This is already true in Flutter tools, so this just canonicalizes that
and modifies the tests to do the same.
To be consistent, hot restart also provides the full URL in the DDC
library bundle format and the bootstrap is modified to reflect that.