Skip to content

[ML] Add test suite for PR pipeline generator - #3197

Open
edsavage wants to merge 2 commits into
elastic:mainfrom
edsavage:add/pr-pipeline-tests
Open

edsavage wants to merge 2 commits into
elastic:mainfrom
edsavage:add/pr-pipeline-tests

Conversation

@edsavage

Copy link
Copy Markdown
Contributor

Summary

Adds dev-tools/unittest/test_pr_pipeline.py to exercise every label- and comment-triggered code path in .buildkite/pipeline.json.py that calls os.environ. The dev-tools pytest CI step already runs this directory, so coverage is automatic on every PR.

Motivated by a missing import os on the 9.4 branch (fixed separately) that caused any backport PR carrying ci:run-qa-tests to crash the pipeline generator before any build step ran.

Test plan

  • All 8 new tests pass locally against the current main branch.
  • Covered paths: no-label default, ci:run-qa-tests, ci:run-pytorch-tests, both together (dedup check), QAF_TESTS_TO_RUN env override, comment-triggered run_qa_tests, comment-triggered run_pytorch_tests, and a parametrised check that every label variant produces steps + env keys.
  • This PR should be backported to active branches (9.5, 9.4, 9.99) so the same protection exists there.

🤖 Generated with Claude Code

@elasticsearchmachine

Copy link
Copy Markdown

Pinging @elastic/ml-core (Team:ML)

@edsavage edsavage added auto-backport Automatically merge backport PRs when CI passes v8.19.22 v9.4.8 v9.5.5 labels Sep 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Tests assert unavailable generator behavior, and the suite is not registered in CI.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds pytest coverage for Buildkite PR pipeline label, comment, and environment-handling paths.

Changes:

  • Adds subprocess-based pipeline generator tests.
  • Covers platform labels, QA/PyTorch triggers, comments, and environment overrides.
File summaries
File Summary
dev-tools/unittest/test_pr_pipeline.py Adds PR pipeline generator tests, including cases for behavior not currently implemented.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread dev-tools/unittest/test_pr_pipeline.py
Comment thread dev-tools/unittest/test_pr_pipeline.py
@edsavage

Copy link
Copy Markdown
Contributor Author

buildkite build

Exercises every label/comment code path in pipeline.json.py that calls
os.environ, so that a missing import (or similar name error) is caught by
the dev-tools pytest CI step before it reaches a real PR build.

The test was prompted by a missing 'import os' on the 9.4 branch that caused
any backport PR carrying ci:run-qa-tests to fail in ~25s before any build
step ran.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@edsavage
edsavage force-pushed the add/pr-pipeline-tests branch from de0b01a to 4c34d9c Compare September 16, 2026 23:24
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants