Started executing vulkan unit tests with validation on macos #42337
Conversation
gaaclarke
force-pushed
the
impeller-unittests-validation-layers
branch
from
125b2d3 to
07910ea
Compare
gaaclarke
changed the title
gaaclarke
force-pushed
the
impeller-unittests-validation-layers
branch
from
07910ea to
7569de1
Compare
| cmd, | ||
| stdout=stdout_pipe, | ||
| stderr=stderr_pipe, | ||
| stdout=subprocess.PIPE, |
There was a problem hiding this comment.
This change allows us to stream the output to stdout but also capture it for inspection. The tests take a long enough time, it's a pain not seeing the streaming results.
|
Here's the output of running the tests to verify the new streaming scheme: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8780019886203678497/+/u/test:_Host_Tests_for_host_debug/stdout |
| sys.stdout.write(line) | ||
|
|
||
| sys.stdout.flush() | ||
| process.wait() |
There was a problem hiding this comment.
Does process.stdout.readline drain the buffer to avoid the deadlock concern with wait() called out in the docs? https://docs.python.org/3/library/subprocess.html#subprocess.Popen.wait
There was a problem hiding this comment.
That's my understanding. It's reading the pipe until EOF so there should be no more waiting on the pipe by the time we call wait.
|
|
||
| for line in iter(process.stdout.readline, ''): | ||
| output += line | ||
| sys.stdout.write(line) |
There was a problem hiding this comment.
Previously the test script would only generate output when a test fails. This is nice when diagnosing failures on CI since the succeeding tests add only minimal output to the logs. Am I misreading what this does? Is there some way to make this configurable?
There was a problem hiding this comment.
I believe you are misremembering how it works. If collect_output == true the output would be printed on line 119. If collect_output == false it would get printed out by virtue of using stdout=sys.stdout.
| 'MTL_SHADER_VALIDATION_TEXTURE_USAGE': | ||
| '1', # Validates that texture references are not nil. | ||
| 'VK_ICD_FILENAMES': os.path.join(build_dir, 'vk_swiftshader_icd.json'), | ||
| 'VK_LAYER_PATH': os.path.join(build_dir, 'vulkan-data'), |
There was a problem hiding this comment.
A comment pointing to the GN file with the string that has to be the same as this one would be good?
gaaclarke
added
the
autosubmit
…127702) flutter/engine@858d975...eed12f3 2023-05-26 [email protected] Started executing vulkan unit tests with validation on macos (flutter/engine#42337) 2023-05-26 [email protected] [Impeller] Avoid encoding metal commands while the GPU is unavailable when decoding images. (flutter/engine#42349) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
2 participants
fixes flutter/flutter#127634
This PR depends on a buildroot change at flutter/buildroot#737
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.