fix(ci): point PR unit-tests at the CDK-managed V3 project - #6082
Merged
jam-jee merged 1 commit intoJul 23, 2026
Merged
Conversation
The PR-check `unit-tests` job invoked per-submodule CodeBuild projects named `sagemaker-python-sdk-ci-<submodule>-unit-tests`. Those projects were created manually (no CloudFormation tags) and are not managed by the SageMakerMLFPySDKInfraCDK pipeline, so they drifted stale: they still run `tox ... --cov=.` (last modified 2026-06-15) even though the merged and deployed buildspec CR changed this to `--cov=sagemaker`. As a result PR coverage kept reporting the old test-file-inflated numbers and never reflected the fix. Point the job at the single CDK/pipeline-managed project `sagemaker-python-sdk-ci-health-unit-test-v3` (createCIUnitV3BuildSpec), driven by the `SUBMODULE` env var, exactly as the ci-health workflow invokes it. This project carries the deployed `--cov=sagemaker` buildspec, so PR coverage now tracks the intended product-only measurement, and the CI wiring stays in sync with the CDK going forward. The manual per-submodule `-unit-tests` projects can be retired separately.
aviruthen
approved these changes
Jul 23, 2026
mohamedzeidan2021
approved these changes
Jul 23, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Issue
The PR-check
unit-testsjob invokes per-submodule CodeBuild projects namedsagemaker-python-sdk-ci-<submodule>-unit-tests. Investigation showed these are orphaned, manually-created projects — not managed by theSageMakerMLFPySDKInfraCDKpipeline:ci-sagemaker-core-unit-tests(PR checks use)ci-health-unit-test-v3(CDK-managed)aws:cloudformation:*presenttox … --cov=.tox … --cov=sagemakerBecause the manual projects aren't pipeline-managed, the deployed buildspec CR (
--cov=.→--cov=sagemaker) never reached them. PR coverage therefore kept reporting the old, test-file-inflated numbers (~90%/sessions=1, 3 files, 37%on merged uploads) instead of the intended product-only coverage. Verified end-to-end: a fresh build on the PR-check project (commit6b41b3f0, run 2026-07-22) still executed--cov=..Fix
Point the
unit-testsjob at the single CDK/pipeline-managed projectsagemaker-python-sdk-ci-health-unit-test-v3(createCIUnitV3BuildSpec), driven by theSUBMODULEenv var — exactly howci-health.ymlalready invokes it. That project carries the deployed--cov=sagemakerbuildspec, so:The
source-version-overrideis preserved so the PR's own code is tested (not master). The matrix still fans out per changed submodule viadetect-changes.Follow-ups (separate)
sagemaker-python-sdk-ci-<submodule>-unit-testsprojects.branch/pr=false/no-Fflags (a separate buildspec issue), which affects whether the report attaches to the PR and whether the 4 sub-uploads merge. Tracked separately.Testing
python -c "import yaml; yaml.safe_load(open('.github/workflows/pr-checks-master.yml'))"→ parses.sagemaker-python-sdk-ci-health-unit-test-v3exists, is CFN-managed, and its deployed buildspec contains--cov=sagemaker.