rptest: add skip_in_cdt opt-out for CDT runs#30975
Conversation
oleiman
added
the
claude-review
There was a problem hiding this comment.
Pull request overview
Adds an opt-out mechanism for ducktape tests to be skipped specifically in CDT (real cloud) runs, keeping local/docker CI behavior unchanged while still reporting skipped tests as IGNORE in CDT.
Changes:
- Introduces
in_cdt(),skip_in_cdt(method/class decorator), andskip_file_in_cdt(module-level helper) inmode_checks.py. - Adds unit tests validating method-, class-, and file-scope skipping behavior and ensuring
reasonis not forwarded to ducktape’signore.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/rptest/utils/mode_checks.py | Adds CDT detection and skip helpers that apply ducktape IGNORE marks only when running in real-cloud CDT. |
| tests/rptest/utils/skip_in_cdt_test.py | Adds unit tests for method/class/file scope CDT skipping and verifies ignore mark semantics. |
| if in_cdt(): | ||
| # ignore(method) attaches IgnoreAll (ignores all parametrizations) in place. | ||
| ignore(method) |
There was a problem hiding this comment.
In-place mutation is ducktape's mark contract (Mark.mark writes to fun.__dict__, same as @cluster/@matrix), so the mark reliably lands on method itself. The method path returns target to keep the type-preserving @overload/TypeVar signature -- capturing ignore()'s loosely-typed return reintroduces strict-pyright errors. I did remove the asymmetry you flagged: both method- and class-scope now go through a shared _mark_own_cdt_tests helper, so neither relies on the return.
oleiman
added
claude-review
ReviewNice, focused utility — gating everything on Main concern: class/file scope misses inherited test methods
Minor notes
Overall the approach is sound; the inherited-test-method gap is the one item I'd want addressed (or at least loudly documented) before this lands, since it can silently fail to skip. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| setattr(cls, "_skip_in_cdt_reason", reason) | ||
| if not in_cdt(): | ||
| return | ||
| for name, member in list(vars(cls).items()): |
There was a problem hiding this comment.
vars(cls) only returns attributes defined directly on cls; inherited test methods are not included. This silently breaks a common rptest pattern where a base class defines the tests and thin subclasses only parametrize:
class TLSVersionTestBase(RedpandaTest):
@cluster(num_nodes=3)
def test_ciphersuite_support(self): ...
@skip_in_cdt(reason="...")
class TLSVersionTestRSA(TLSVersionTestBase): # no own test methods
...Here vars(TLSVersionTestRSA) contains no test methods, so the decorator marks nothing and the test still runs in CDT — exactly the failure mode this PR is trying to prevent, with no error to signal it.
skip_file_in_cdt has the same gap whenever the base class lives in a different module (the __module__ filter excludes it, and the subclass's vars() is empty). It happens to work only when the base is defined in the same file, because the mark lands on the shared function object.
Consider walking the MRO (e.g. iterate over dir(cls) / inspect.getmembers, or cls.__mro__) so inherited test methods are also covered. Note the symmetric over-skip risk if you mark a shared base function: it would ignore that test for all subclasses. At minimum, document this limitation prominently in the docstring.
There was a problem hiding this comment.
Good catch -- the silent no-op was the real hazard. Since ducktape marks are function-scoped, walking the MRO to mark inherited methods would trade under-skip for over-skip (it would skip the shared base test for every subclass). So class-scope now raises when a class's tests are inherited, pointing to method- or base-scope. skip_file_in_cdt stays lenient: it marks tests defined in the module, so a same-module base still covers its subclasses via the shared function object; the cross-module case is documented. Added tests for both the raise and the same-module coverage.
CDT runs every ducktape test on real cloud infrastructure, and a whole-run timeout fails the entire run. Some tests (e.g. exhaustive broker HTTP-API tests) add runtime there without adding cloud-infra coverage. skip_in_cdt opts out a test method or a whole class; skip_file_in_cdt opts out every test class in a module. All are gated on CLOUD_PROVIDER != "docker", so they no-op locally and in dockerized CI and the default behavior is unchanged. In CDT they attach ducktape's ignore mark, so opted-out tests report as IGNORE rather than vanishing from the run. reason is documentation only; it is never passed to ducktape's ignore, which would otherwise treat it as a parametrization matcher and silently un-skip the test.
oleiman
force-pushed
the
ci/noticket/opt-out-cdt
branch
from
c2a250c to
d99bbc5
Compare
|
/cdt |
|
/cdt |
|
/cdt |
oleiman
force-pushed
the
ci/noticket/opt-out-cdt
branch
from
2379a47 to
d99bbc5
Compare
|
/cdt |
CDT runs every ducktape test on real cloud infrastructure, and a whole-run timeout fails the entire run. Some tests (e.g. exhaustive broker HTTP-API tests) add runtime there without adding cloud-infra coverage.
skip_in_cdt opts out a test method or a whole class; skip_file_in_cdt opts out every test class in a module. All are gated on CLOUD_PROVIDER != "docker", so they no-op locally and in dockerized CI and the default behavior is unchanged. In CDT they attach ducktape's ignore mark, so opted-out tests report as IGNORE rather than vanishing from the run.
reason is documentation only; it is never passed to ducktape's ignore, which would otherwise treat it as a parametrization matcher and silently un-skip the test.
Accounting
Backports Required
Release Notes