Skip to content
Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion projects/hip-tests/TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ When you change public HIP API behavior, run the contract tests for the touched

A flaky or quarantined case is treated as unstable signal: it does not block merge, but it must have an owner, a tracking issue, and a removal or fix plan before it can be promoted back to trusted signal. Use `HIP_SKIP_TEST` for a genuine capability gap such as an unsupported device or runtime path, too few GPUs, or no image support, so the case skips cleanly instead of failing. Never leave a test that crashes the process. Any test that intentionally triggers a HIP error must consume the sticky thread-local error with `(void)hipGetLastError()` before returning so it does not leak into later cases in the same process.

Disabling a test case should be rare and explicit. It is appropriate when a test cannot execute on a platform or architecture because a required capability, OS facility, graphics environment, or runtime mode is absent; when a known product or infrastructure defect has a tracking issue; or when keeping the test enabled would crash the process and hide later signal. Prefer runtime capability checks plus `HIP_SKIP_TEST` for environment-dependent behavior, and use YAML `disabled:` entries only when a case should be hidden for a specific platform, architecture, sanitizer mode, or tracked known issue. Do not disable a test only to make CI green. Every disabled or quarantined case should record why it is disabled, where it is tracked, and what condition allows it to be re-enabled.
Disabling a test case should be rare and explicit. It is appropriate when a test cannot execute on a platform or architecture because a required capability, OS facility, graphics environment, or runtime mode is absent; when a known product or infrastructure defect has a tracking issue; or when keeping the test enabled would crash the process and hide later signal. Prefer runtime capability checks plus `HIP_SKIP_TEST` for environment-dependent behavior, and use the YAML skip fields only when a case should be hidden for a specific platform, architecture, sanitizer mode, or tracked known issue. The `disabled:` and `unsupported:` fields share the same syntax and produce the same skip; they differ only in intent. Do not disable a test only to make CI green. Every skipped case should record why it is skipped, where it is tracked when a `disabled:` entry points at a bug.

## Coverage

Expand Down
12 changes: 9 additions & 3 deletions projects/hip-tests/catch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ All configuration for the test cases is done through the `hip_tests_config.yaml`
Every test case has its own entry. Currently supported options are:
- level : Specify to which level the case belongs to (e.g. Level_2 is a standard test)
- tags : List all Catch2 tags that the case is associated with
- disabled : List all platforms where the case should be disabled
- disabled : Temporarily skip the case on the listed targets (a regression or tracked bug that is expected to be re-enabled)
- unsupported : Permanently skip the case on the listed targets, where the platform, architecture, or backend cannot support it
`disabled` and `unsupported` share the same syntax and produce the same skip; they differ only in intent (temporary regression versus permanent capability gap).
The group name is automatically added as a tag for every case.
Changing the configuration file will retrigger the build, so we have an up to date configuration every time.

Expand All @@ -80,8 +82,12 @@ unit:
Unit_atomicExch_system_Positive_Peer_GPUs:
<<: *level_2
tags: [multigpu]
# SWDEV-435667: Below tests failing randomly in stress test on 01/12/23
disabled: [amd_wsl]
disabled:
targets: [amd_wsl]
reason: "SWDEV-435667: fails randomly in stress test"
unsupported:
targets: [gfx900]
reason: requires a hardware feature not present on this architecture
```
will be generated (on an AMD linux machine) as:
```cpp
Expand Down
27 changes: 27 additions & 0 deletions projects/hip-tests/catch/config/check_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,29 @@ def main():
configs_path = args.configs_path

missing = []
invalid_skip_fields = []

for group, cases in iter_group_configs(configs_path):
for case_name, case_config in cases.items():
if "level" not in case_config:
missing.append(f" {group}/{case_name}")
for field in ("disabled", "unsupported"):
if field not in case_config:
continue
value = case_config[field]
# A skip field is either a flat list of targets, or a mapping
# with a 'targets' list plus an optional 'reason' scalar. The
# reason is metadata and is intentionally NOT enforced yet —
# deferred to a later phase (AIRUNTIME-2744):
# https://amd-hub.atlassian.net/browse/AIRUNTIME-2744
# We only guard that the targets are a list, since a scalar
# breaks tag generation (a string is iterated char-by-char) and
# a malformed mapping fails the parser's list concatenation.
if isinstance(value, list):
continue
if isinstance(value, dict) and isinstance(value.get("targets"), list):
continue
invalid_skip_fields.append(f" {group}/{case_name}: '{field}'")

if missing:
print(
Expand All @@ -41,6 +59,15 @@ def main():
print(f"[check_config] {ERROR}{entry}{RESET}", file=sys.stderr)
sys.exit(1)

if invalid_skip_fields:
print(
f"[check_config] {ERROR}ERROR: The following test cases have a 'disabled'/'unsupported' field that is neither a YAML list nor a mapping with a 'targets' list:{RESET}",
file=sys.stderr,
)
for entry in invalid_skip_fields:
print(f"[check_config] {ERROR}{entry}{RESET}", file=sys.stderr)
sys.exit(1)


if __name__ == "__main__":
main()
32 changes: 27 additions & 5 deletions projects/hip-tests/catch/config/parse_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,39 @@ def parse_args():
"--asan",
action="store_true",
help="Target is an Address Sanitizer (ASAN) build. Test cases that "
"list 'asan' in their 'disabled' field are skipped.",
"list 'asan' in their 'disabled' or 'unsupported' field are skipped.",
)
return parser.parse_args()


def _skip_targets(field):
"""Return the platform/arch/config targets from a skip field.

A skip field is either a flat list of targets ([amd_windows]) or a mapping
with a 'targets' list plus an optional 'reason' scalar
({targets: [...], reason: ...}). Only the targets drive tag generation; the
reason is metadata for other tooling and is ignored here.
"""
if isinstance(field, dict):
return field.get("targets", [])
return field


def create_test_definition(
group, case_name, case_config, platform, os_name, arch, asan=False
):
level = case_config.get("level", 2)
tags = case_config.get("tags", [])
disabled = case_config.get("disabled", [])
unsupported = case_config.get("unsupported", [])
# Extract the targets from each skip field (list form or {targets, reason}
# mapping form) and merge into one ordered local (disabled first) so the
# match and promotion logic below is identical for either field. Do not
# mutate the source lists. "disabled" is temporary/regressions, "unsupported"
# is permanent; both produce the same [disabled] skip tag and
# [exclude_<entry>] promotions that CI and the compute-utils/WSL runners
# depend on.
skip_targets = _skip_targets(disabled) + _skip_targets(unsupported)
Comment thread
sruscica marked this conversation as resolved.
Resolved

tags_str = ""

Expand All @@ -62,9 +84,9 @@ def create_test_definition(
tags_str += f"[{group}]"

if (
f"{platform}_{os_name}" in disabled
or arch in disabled
or (asan and "asan" in disabled)
f"{platform}_{os_name}" in skip_targets
or arch in skip_targets
or (asan and "asan" in skip_targets)
):
# Disabled on this platform (e.g. amd_linux) or arch (e.g. gfx1260).
# Use the [disabled] tag (no leading dot) so it is visible in --list-tests
Expand All @@ -78,7 +100,7 @@ def create_test_definition(
# specific labels a test is disabled for (incl. OS labels dropped above).
# Prefix is "exclude_" (not "disabled_") so it does not substring-match a
# ctest -LE disabled filter; consumers should match anchored ^exclude_<entry>$.
for entry in disabled:
for entry in skip_targets:
tags_str += f"[exclude_{entry}]"

return f'#define {case_name} "{case_name}", "{tags_str}"'
Expand Down
Loading