[TEST] No-trigger test: tests/e2e/ change - #3008
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merge Protections🔴 2 of 2 protections blocking · waiting on 👀 reviews
🔴 Require one maintainer reviewWaiting for any of
This rule is failing.All PRs must have at least one approving review from a maintainer before merging.
🔴 Require two reviewsWaiting for
This rule is failing.PRs labelled "two-reviews" must have at least two approving reviews before merging.
|
There was a problem hiding this comment.
Code Review
This pull request simplifies the Buildkite pipeline configuration for H100 and L4 GPU tests by replacing manual git diff shell scripts with native Buildkite if_changed conditions, splitting the transformers tests into separate branch-push and PR-specific steps. The reviewer identified a critical issue with the conditional checks: in Buildkite, build.pull_request evaluates to false rather than null on branch builds. As a result, using == null and != null will cause incorrect step execution behavior, and these checks should be updated to compare against false instead.
| - label: ":mag: Detect changes for Transformers Tests" | ||
| # Transformers tests - always run on branch pushes | ||
| - label: ":mag: Transformers Tests (branch)" | ||
| if: build.pull_request == null |
There was a problem hiding this comment.
In Buildkite, build.pull_request evaluates to false (boolean) when the build is not a pull request, rather than null. Using build.pull_request == null will evaluate to false on branch builds, causing this step to be skipped. Please use build.pull_request == false instead.
if: build.pull_request == false|
|
||
| # Transformers tests - conditional on PRs when relevant paths changed | ||
| - label: ":mag: Transformers Tests (PR)" | ||
| if: build.pull_request != null |
| - label: ":mag: Detect changes for Transformers Tests" | ||
| # Transformers tests - always run on branch pushes | ||
| - label: ":mag: Transformers Tests (branch)" | ||
| if: build.pull_request == null |
There was a problem hiding this comment.
In Buildkite, build.pull_request evaluates to false (boolean) when the build is not a pull request, rather than null. Using build.pull_request == null will evaluate to false on branch builds, causing this step to be skipped. Please use build.pull_request == false instead.
if: build.pull_request == false|
|
||
| # Transformers tests - conditional on PRs when relevant paths changed | ||
| - label: ":mag: Transformers Tests (PR)" | ||
| if: build.pull_request != null |
|
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
e384ca3 to
7ec4e8d
Compare
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
… regex operator Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
7ec4e8d to
bcbccb3
Compare
Test PR to verify if_changed exclusions
This PR modifies a file in
tests/e2e/which should NOT TRIGGER transformers tests (excluded).Part of verifying the if_changed include/exclude pattern in #2983