Skip to content

Fixes a flake in reload shaders tests#184268

Open
gaaclarke wants to merge 2 commits intoflutter:masterfrom
gaaclarke:reload-shader-flake
Open

Fixes a flake in reload shaders tests#184268
gaaclarke wants to merge 2 commits intoflutter:masterfrom
gaaclarke:reload-shader-flake

Conversation

@gaaclarke
Copy link
Copy Markdown
Member

@gaaclarke gaaclarke commented Mar 27, 2026

fixes #184265

This fixes an oddity related to mmap files. A delay had to be introduced in the test because of oddities with the filesystem. I tried a few tricks to get it to work without the delay but this is the only way I could get it to work.

testing: has existing test, removes a flake

I tested it manually locally:

for i in {1..30}; do echo "Running iteration: $i"; ./testing/run_tests.py --type=dart --dart-filter=shader_reload_test.dart --variant host_debug_unopt_arm64; done

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Mar 27, 2026
@gaaclarke gaaclarke added the CICD Run CI/CD label Mar 27, 2026
@gaaclarke gaaclarke marked this pull request as ready for review March 27, 2026 22:33
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 modifies shader_reload_test.dart by introducing a _deleteAndWrite helper function to update shader files, intended to prevent the engine from accessing memory-mapped files during truncation. A review comment suggests using an atomic write pattern with a temporary file and rename operation to eliminate a potential race condition where the file is briefly non-existent.

@github-actions github-actions bot removed the CICD Run CI/CD label Mar 27, 2026
@gaaclarke gaaclarke added the CICD Run CI/CD label Mar 27, 2026
@gaaclarke gaaclarke requested review from b-luk and walley892 March 27, 2026 23:17
Copy link
Copy Markdown
Contributor

@b-luk b-luk left a comment

Choose a reason for hiding this comment

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

LGTM, but I wonder why Gemini's suggestion doesn't work. From your error message, it looks like the move fails because the temp file doesn't exist. I wouldn't expect that to error to happen.

@gaaclarke
Copy link
Copy Markdown
Member Author

LGTM, but I wonder why Gemini's suggestion doesn't work. From your error message, it looks like the move fails because the temp file doesn't exist. I wouldn't expect that to error to happen.

Yea, that was a surprise to me too. Maybe we should file a Dart bug. I shouldn't have to accommodate for this at the app layer if I'm writing synchronously and flushing.

@gaaclarke
Copy link
Copy Markdown
Member Author

I'm lead to believe this is somehow related to using mmap. Maybe that bypasses some filesystem synchronization that happens.

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 27, 2026
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 28, 2026
@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit bot commented Mar 28, 2026

autosubmit label was removed for flutter/flutter/184268, because - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

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

Labels

CICD Run CI/CD engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mac mac_unopt flaking on reorder samplers integration test

2 participants