Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Ignore some MTLCompiler failures in impeller unit tests #40391

Merged
merged 1 commit into from
Mar 17, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 55 additions & 29 deletions testing/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
allowed_failure_output=None,
failure_allow_list=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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
collect_output = forbidden_output or allowed_failure_output
should_collect_output = forbidden_output or allowed_failure_output

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,
Expand Down Expand Up @@ -92,10 +100,17 @@ def run_cmd(

print_divider('!')

raise Exception(
'Command "%s" exited with code %d.' %
(command_string, process.returncode)
)
allowed_failure = False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
allowed_failure = False
is_allowed_failure = False

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:
Copy link
Member

Choose a reason for hiding this comment

The 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)
Expand Down Expand Up @@ -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,
Expand All @@ -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 = {}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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():
Expand Down