This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Ignore some MTLCompiler failures in impeller unit tests #40391
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -52,19 +52,27 @@ def is_asan(build_dir): | |||||
|
||||||
|
||||||
def run_cmd( | ||||||
cmd, forbidden_output=None, expect_failure=False, env=None, **kwargs | ||||||
cmd, | ||||||
forbidden_output=None, | ||||||
expect_failure=False, | ||||||
env=None, | ||||||
allowed_failure_output=None, | ||||||
**kwargs | ||||||
): | ||||||
if forbidden_output is None: | ||||||
forbidden_output = [] | ||||||
if allowed_failure_output is None: | ||||||
allowed_failure_output = [] | ||||||
|
||||||
command_string = ' '.join(cmd) | ||||||
|
||||||
print_divider('>') | ||||||
print('Running command "%s"' % command_string) | ||||||
|
||||||
start_time = time.time() | ||||||
stdout_pipe = sys.stdout if not forbidden_output else subprocess.PIPE | ||||||
stderr_pipe = sys.stderr if not forbidden_output else subprocess.PIPE | ||||||
collect_output = forbidden_output or allowed_failure_output | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
stdout_pipe = sys.stdout if not collect_output else subprocess.PIPE | ||||||
stderr_pipe = sys.stderr if not collect_output else subprocess.PIPE | ||||||
process = subprocess.Popen( | ||||||
cmd, | ||||||
stdout=stdout_pipe, | ||||||
|
@@ -92,10 +100,17 @@ def run_cmd( | |||||
|
||||||
print_divider('!') | ||||||
|
||||||
raise Exception( | ||||||
'Command "%s" exited with code %d.' % | ||||||
(command_string, process.returncode) | ||||||
) | ||||||
allowed_failure = False | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
for allowed_string in allowed_failure_output: | ||||||
if (stdout and allowed_string in stdout) or (stderr and | ||||||
allowed_string in stderr): | ||||||
allowed_failure = True | ||||||
|
||||||
if not allowed_failure: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably log allowed failures, right? |
||||||
raise Exception( | ||||||
'Command "%s" exited with code %d.' % | ||||||
(command_string, process.returncode) | ||||||
) | ||||||
|
||||||
if stdout or stderr: | ||||||
print(stdout) | ||||||
|
@@ -192,6 +207,7 @@ def run_engine_executable( # pylint: disable=too-many-arguments | |||||
flags=None, | ||||||
cwd=BUILDROOT_DIR, | ||||||
forbidden_output=None, | ||||||
allowed_failure_output=None, | ||||||
expect_failure=False, | ||||||
coverage=False, | ||||||
extra_env=None, | ||||||
|
@@ -205,6 +221,8 @@ def run_engine_executable( # pylint: disable=too-many-arguments | |||||
flags = [] | ||||||
if forbidden_output is None: | ||||||
forbidden_output = [] | ||||||
if allowed_failure_output is None: | ||||||
allowed_failure_output = [] | ||||||
if extra_env is None: | ||||||
extra_env = {} | ||||||
|
||||||
|
@@ -249,7 +267,8 @@ def run_engine_executable( # pylint: disable=too-many-arguments | |||||
cwd=cwd, | ||||||
forbidden_output=forbidden_output, | ||||||
expect_failure=expect_failure, | ||||||
env=env | ||||||
env=env, | ||||||
allowed_failure_output=allowed_failure_output | ||||||
) | ||||||
except: | ||||||
# The LUCI environment may provide a variable containing a directory path | ||||||
|
@@ -287,6 +306,7 @@ def __init__( # pylint: disable=too-many-arguments | |||||
flags=None, | ||||||
cwd=BUILDROOT_DIR, | ||||||
forbidden_output=None, | ||||||
allowed_failure_output=None, | ||||||
expect_failure=False, | ||||||
coverage=False, | ||||||
extra_env=None, | ||||||
|
@@ -297,6 +317,7 @@ def __init__( # pylint: disable=too-many-arguments | |||||
self.flags = flags | ||||||
self.cwd = cwd | ||||||
self.forbidden_output = forbidden_output | ||||||
self.allowed_failure_output = allowed_failure_output | ||||||
self.expect_failure = expect_failure | ||||||
self.coverage = coverage | ||||||
self.extra_env = extra_env | ||||||
|
@@ -309,6 +330,7 @@ def __call__(self, *args): | |||||
flags=self.flags, | ||||||
cwd=self.cwd, | ||||||
forbidden_output=self.forbidden_output, | ||||||
allowed_failure_output=self.allowed_failure_output, | ||||||
expect_failure=self.expect_failure, | ||||||
coverage=self.coverage, | ||||||
extra_env=self.extra_env, | ||||||
|
@@ -442,28 +464,32 @@ def make_test(name, flags=None, extra_env=None): | |||||
shuffle_flags, | ||||||
coverage=coverage | ||||||
) | ||||||
# TODO(117122): Re-enable impeller_unittests after shader compiler errors | ||||||
# are addressed. | ||||||
# Impeller tests are only supported on macOS for now. | ||||||
# run_engine_executable( | ||||||
# build_dir, | ||||||
# 'impeller_unittests', | ||||||
# executable_filter, | ||||||
# shuffle_flags, | ||||||
# coverage=coverage, | ||||||
# extra_env={ | ||||||
# # pylint: disable=line-too-long | ||||||
# # See https://developer.apple.com/documentation/metal/diagnosing_metal_programming_issues_early?language=objc | ||||||
# 'MTL_SHADER_VALIDATION': | ||||||
# '1', # Enables all shader validation tests. | ||||||
# 'MTL_SHADER_VALIDATION_GLOBAL_MEMORY': | ||||||
# '1', # Validates accesses to device and constant memory. | ||||||
# 'MTL_SHADER_VALIDATION_THREADGROUP_MEMORY': | ||||||
# '1', # Validates accesses to threadgroup memory. | ||||||
# 'MTL_SHADER_VALIDATION_TEXTURE_USAGE': | ||||||
# '1', # Validates that texture references are not nil. | ||||||
# } | ||||||
# ) | ||||||
run_engine_executable( | ||||||
build_dir, | ||||||
'impeller_unittests', | ||||||
executable_filter, | ||||||
shuffle_flags, | ||||||
coverage=coverage, | ||||||
extra_env={ | ||||||
# pylint: disable=line-too-long | ||||||
# See https://developer.apple.com/documentation/metal/diagnosing_metal_programming_issues_early?language=objc | ||||||
'MTL_SHADER_VALIDATION': | ||||||
'1', # Enables all shader validation tests. | ||||||
'MTL_SHADER_VALIDATION_GLOBAL_MEMORY': | ||||||
'1', # Validates accesses to device and constant memory. | ||||||
'MTL_SHADER_VALIDATION_THREADGROUP_MEMORY': | ||||||
'1', # Validates accesses to threadgroup memory. | ||||||
'MTL_SHADER_VALIDATION_TEXTURE_USAGE': | ||||||
'1', # Validates that texture references are not nil. | ||||||
}, | ||||||
# TODO(117122): Remove this allowlist. | ||||||
# https://github.com/flutter/flutter/issues/114872 | ||||||
allowed_failure_output=[ | ||||||
'[MTLCompiler createVertexStageAndLinkPipelineWithFragment:', | ||||||
'[MTLCompiler pipelineStateWithVariant:', | ||||||
] | ||||||
) | ||||||
|
||||||
|
||||||
def parse_impeller_vulkan_filter(): | ||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.