Skip to content

test(serve): Add skip_in_pr_check marker for hang-prone integ tests - #6190

Merged
lucasjia-aws merged 1 commit into
aws:masterfrom
lucasjia-aws:slow-tests
Aug 18, 2026
Merged

test(serve): Add skip_in_pr_check marker for hang-prone integ tests#6190
lucasjia-aws merged 1 commit into
aws:masterfrom
lucasjia-aws:slow-tests

Conversation

@lucasjia-aws

Copy link
Copy Markdown
Collaborator

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Claude finished @lucasjia-aws's task in 1m 22s —— View job


Review complete ✅

This PR registers a new skip_in_pr_check pytest marker and applies it to three hang-prone / long-running integ tests. The changes are small, correct, and follow the existing convention used for gpu_intensive / import_model.

Task list

  • Read PR diff (/tmp/pr.diff)
  • Check how markers are consumed by CI / tox
  • Verify pytest config precedence and strict-markers behavior
  • Post review

Findings

No blocking issues. The three decorator additions (test_ai_inference_recommender_integration.py:73, test_model_customization_deployment.py:110, test_optimize_integration.py:42) are all applied cleanly, and the goal — excluding these tests from PR checks via pytest -m "not skip_in_pr_check" — works regardless of registration.

1. tox.ini marker registration is effectively dead code (informational)

sagemaker-serve/pyproject.toml defines [tool.pytest.ini_options], and sagemaker-serve/tox.ini defines [pytest]. Pytest reads config from only the first file found in its precedence order (pytest.inipyproject.toml [tool.pytest.ini_options]tox.ini [pytest]setup.cfg). Since pyproject.toml already contains [tool.pytest.ini_options], the entire [pytest] section in tox.ini — including the newly added skip_in_pr_check line and all the pre-existing markers (slow_test, gpu_intensive, import_model, etc.) — is never read by pytest.

Practical consequences:

  • The tox.ini line (+1) is redundant; the marker is actually registered by the new pyproject.toml markers = [...] block (pyproject.toml:76-78).
  • This is a pre-existing situation, not a regression from this PR — the other markers were already only declared in tox.ini and thus already unregistered from pytest's perspective. Because there is no --strict-markers or filterwarnings = error in the config, unregistered markers only emit PytestUnknownMarkWarning rather than failing.
  • If you'd like this to be robust (e.g., someone later enables --strict-markers), consider consolidating all markers into the pyproject.toml markers list rather than splitting the source of truth across two files — or, at minimum, dropping the tox.ini addition since it has no effect.

2. Marker selection works either way

Even though registration lives in pyproject.toml, -m "not skip_in_pr_check" selection works whether or not the marker is registered, so the CI filtering (presumably in the CodeBuild buildspec, outside this repo) will behave as intended. Registration here just silences the warning — which is the right thing to do.

No security, correctness, or performance concerns. No missing tests (this is test infrastructure only).

Note: inline comments could not be posted (the inline-comment tool was unavailable in this run), so findings are consolidated here.

@lucasjia-aws
lucasjia-aws merged commit fd775cd into aws:master Aug 18, 2026
14 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants