Skip to content

Commit c84bc44

Browse files
smackeseyDagster Devtools
authored andcommitted
buildkite: consolidate lints into monorepo-integrity-tests (#25258)
## Summary & Motivation The `buildkite-lints` Buildkite step (`dagster-oss/.buildkite/buildkite-shared/lints.py`) was running five pytest-style checks on every PR. Three of those checks already exist verbatim in `integration_tests/monorepo-integrity-tests/conventions_tests/` — `test_tox_passenv`, `test_no_subdirectory_pytest_ini`, `test_no_tox_pytest_config_override` — and the `conventions_tests` versions run repo-wide via `git ls-files` rather than the dagster-oss-only `os.walk(".")` scan, so they're a strict superset. The remaining two — `test_tox_has_lockfile` and `test_pyproject_has_requires_python` — had no counterpart in `conventions_tests` despite being conceptually identical in spirit. This PR moves the two unique checks into `conventions_tests/` and deletes `lints.py` and the `buildkite-lints` step entirely, consolidating all pytest-shaped repo-wide checks into one place. ## Changes - `conventions_tests/test_tox_and_pytest_configuration.py`: added `test_dagster_oss_tox_has_lockfile`. Refactored the existing fixtures so `tox_config` now depends on a new `tox_ini_path` fixture, and added a sibling `dagster_oss_tox_ini_path` fixture (scoped to `dagster-oss/**/tox.ini`) that the new test consumes. Inlined the `_is_gitignored` helper for the uv.lock opt-out path. - `conventions_tests/test_package_structure.py`: added `test_dagster_oss_project_pyprojects_have_requires_python`, following the aggregated-violations pattern already used by `test_all_oss_packages_have_py_typed` in that file. - Deleted `dagster-oss/.buildkite/buildkite-shared/lints.py`. - Removed `build_buildkite_lint_steps()` and its call in `build_repo_wide_steps()` from `dagster-oss/.buildkite/dagster-buildkite/dagster_buildkite/steps/dagster.py`. ## Scope of the moved tests Both moved tests are scoped to `dagster-oss/` to match the prior `lints.py` behavior — they're not expanded to the whole internal repo. Expanding would require fixing ~30 internal pyprojects that lack `requires-python` and most internal toxfiles that aren't on `uv-venv-lock-runner`; that's a much larger separate cleanup. ## Test Plan - `cd integration_tests/monorepo-integrity-tests && tox -e py312-conventions` — 454 tests pass (6.7s), including the two newly-added ones across all dagster-oss toxfiles and pyprojects. - `just ruff` clean. - Grep confirmed no other references to `lints.py`, `buildkite-lints`, or `build_buildkite_lint_steps` in the repo (internal `.buildkite/`, dagster-oss `.buildkite/`, `integration_tests/`). Internal-RevId: d53c5c53211517f6af3f9bf90d35265ba2429741
1 parent db52909 commit c84bc44

2 files changed

Lines changed: 0 additions & 171 deletions

File tree

.buildkite/buildkite-shared/lints.py

Lines changed: 0 additions & 157 deletions
This file was deleted.

.buildkite/dagster-buildkite/dagster_buildkite/steps/dagster.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,13 @@
3030
from dagster_buildkite.steps.test_project import build_test_project_steps
3131

3232

33-
def build_buildkite_lint_steps() -> list[CommandStepConfiguration]:
34-
commands = [
35-
f"cd {oss_path('.')}",
36-
"pytest .buildkite/buildkite-shared/lints.py",
37-
]
38-
return [
39-
CommandStepBuilder("buildkite-lints", [":lint-roller:"])
40-
.run(*commands)
41-
.on_test_image()
42-
.build()
43-
]
44-
45-
4633
def build_repo_wide_steps(ctx: BuildkiteContext) -> list[StepConfiguration]:
4734
# Other linters may be run in per-package environments because they rely on the dependencies of
4835
# the target. `ty` and `ruff` are run for the whole repo at once.
4936
return [
5037
*build_repo_wide_ty_steps(ctx),
5138
*build_repo_wide_ruff_steps(ctx),
5239
*build_repo_wide_prettier_steps(ctx),
53-
*build_buildkite_lint_steps(),
5440
]
5541

5642

0 commit comments

Comments
 (0)