Reland: don't update last compile time when compilation is rejected.#41580
Reland: don't update last compile time when compilation is rejected.#41580jonahwilliams merged 12 commits intoflutter:masterfrom
Conversation
fluttergithubbot
added
the
tool
|
|
||
| test('newly added code executes during hot reload', () async { | ||
| await _flutter.run(); | ||
| if (Platform.isWindows) { |
There was a problem hiding this comment.
My current guess is that slight changes in the recorded timestamp have tripped up the windows filesystem. Since the files are modified synchronously in the test environment, perhaps the last recorded timestamp is identical to the first compiled time?
I tried a few different approaches like keeping the datetime initialization before the call to compile and only rolling it back afterwards, but that wasn't sufficient.
There was a problem hiding this comment.
Ahh fails on CI, passes locally ... more time?
There was a problem hiding this comment.
I moved the creation of the lastCompiledTime all the way back to the start of the method... that plus the awaits should help....
There was a problem hiding this comment.
Removing the awaits, that was all that we needed.
There was a problem hiding this comment.
The issue was likely that moving DateTime.now() to after the compilation placed it closed enough to the next invalidation that the with the windows file-system the timestamp granularity was not sufficient.
jonahwilliams
changed the title
Codecov Report
@@ Coverage Diff @@
## master #41580 +/- ##
==========================================
+ Coverage 60.52% 60.61% +0.08%
==========================================
Files 193 192 -1
Lines 18891 18784 -107
==========================================
- Hits 11434 11385 -49
+ Misses 7457 7399 -58
Continue to review full report at Codecov.
|
| ); | ||
|
|
||
| expect(report.success, false); | ||
| expect(devFS.lastCompiled, previousCompile); |
There was a problem hiding this comment.
So this test checks that the correct value of lastCompiled is retained, but it would probably also be good to have a test that demonstrates why retaining the correct value is necessary.
There was a problem hiding this comment.
I've updated with a test case that confirms that the lastCompiled time is updated on success. Correctly using that value is a bit out of scope, since that is only used by the resident_runner
…lutter#41580) * dont update last compiled time when compilation is rejected * lets try flushing, thats a neat trick * windows man * Update hot_reload_test.dart * Update hot_reload_test.dart * Update devfs.dart * Update hot_reload_test.dart * Update hot_reload_test.dart * add test that verifies when compile is good that time is updated * Update devfs_test.dart
4 participants
Description
If we reject the compilation, keep the source files invalidated until they are updated. This keeps the error if the user repeatedly recompiles with an exception
Fixes #41399