feat: run edx-platform + plugin tests together in platform test (+ lehrer build facade)#111
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new plugin-regression verification command to the lehrer build tool, allowing operators to run installed plugins' own pytest suites against a deployment's platform build and settings using pytest discovery. The review feedback highlights a critical issue where packages listed in packages_to_remove are not filtered out of plugin_dists and extra_specs, potentially causing them to be re-installed or to fail the regression script. Additionally, suggestions were made to normalize target distribution names within the regression script to avoid casing or separator mismatches, and to import the corresponding normalization helper.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
Adds plugin regression testing and reorganizes the build CLI into a cohesive build/verify facade.
Changes:
- Adds installed-plugin pytest discovery with optional test extras and JUnit output.
- Consolidates cell-scoped CLI commands and groups commands by purpose.
- Adds tests and deployment documentation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/lehrer/core/plugin_regression.py |
Implements plugin selection and pytest driver generation. |
src/lehrer/core/platform.py |
Adds the Dagger plugin-regression command. |
src/lehrer/cli/build.py |
Groups commands and shares cell argument handling. |
tests/core/test_plugin_regression.py |
Tests plugin regression helpers. |
tests/cli/test_build.py |
Tests CLI command forwarding. |
deployments/mit-ol/build.md |
Documents plugin regression usage. |
3 tasks
Add the verification-pyramid tier above `check_deployment` (install + import): `platform plugin-regression` runs the installed plugins' OWN pytest suites against the deployment's platform build and derived `lehrer_test` settings, so a plugin bump — or a new edx-platform commit — that breaks a plugin's tests surfaces in lehrer rather than in production. Strategy is pytest discovery, not source acquisition. Published plugin wheels/sdists ship no test suites and the monorepo many plugins live in has no per-package tags, so "tests at the exact installed version" isn't something the ecosystem supports. Instead we run whatever tests are installed in the image: with --install-test-extras (default) maintained `ol-*` plugins are re-requested at their pinned version with a `[tests]` extra (a safe no-op until they define one), and dist->module is resolved at runtime from importlib metadata — the same drift-proof approach as the import check. A plugin that ships no tests is reported and skipped, so this passes cleanly today and starts exercising real suites as the extras land. Results aggregate into one JUnit report. New pure module `plugin_regression.py` (extras-spec selection + the in-container discovery driver) with unit tests; the Dagger `plugin_regression` function mirrors `test`'s proven install path (verified to register + type-check via `dagger call platform plugin-regression --help`). Also make `lehrer build` a usable facade: group the command tree into Build / Verify / Utilities so `--help` reads as the mental model, share one cell-forwarding helper across platform/check/test/plugin-regression (they can't drift), and factor the repeated cell argument into `CellArg`. Adds `tests/cli/test_build.py` and a build.md section for the new target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review direction: `platform test` should run the upstream app suite AND the installed plugins' own suites together, not have plugins in a separate target. So the standalone `plugin_regression` Dagger function + CLI command are dropped, and their discovery is folded into `platform test`: - `test` gains `--include-plugins` (default true) and `--install-test-extras` (default true). When including plugins, it derives the plugin distributions from the same requirement files install_deps used, installs each maintained `ol-*` plugin's pinned `[tests]` extra (safe no-op until defined), and runs ONE pytest over the edx-platform paths plus the installed plugin packages (appended via `--pyargs`, resolved at runtime), producing one aggregated JUnit report. `--no-include-plugins` runs the edx-platform suite alone. - `plugin_regression.py` -> `plugin_tests.py`: keeps `maintained_test_extra_specs` and replaces the plugin-only driver with `combined_pytest_script`, which interleaves edx paths (filesystem) and plugin packages (import names) in one run — verified pytest's `--pyargs` only reinterprets non-path args, so both coexist. - CLI keeps the Build/Verify/Utilities facade; the `test` wrapper documents the new flags. Docs + tests updated (test_plugin_tests.py, test_build.py). Verified: ruff/format/mypy/full pre-commit pass; 246 pytest pass; `dagger call platform test --help` shows the new flags and `plugin-regression` is gone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
blarghmatey
force-pushed
the
ci/plugin-regression-runner
branch
from
July 17, 2026 17:09
7f9fbc9 to
6b01cc6
Compare
blarghmatey
changed the title
platform test (+ lehrer build facade)
…ide pins, wildcards, JUnit claim) Gemini + Copilot review on #111: - packages_to_remove: install_deps uninstalls those packages, but the plugin targets and `[tests]` extras were derived from the unfiltered requirement files — so a removed `ol-*` plugin would be reinstalled by its extra and/or handed to pytest, diverging from the production cell. Now exclude normalized removals from both `plugin_dists` and `extra_specs`. - Override pins: `maintained_test_extra_specs` kept the FIRST pin, but a cell's overrides are concatenated after its package list and install_deps applies them last — so the extra could downgrade the plugin. Now LAST pin wins its version while keeping first-seen order; test updated. - Wildcard pins: the `==` regex accepted `==1.2.*`, which could move the patch. Version class now excludes `*`. - JUnit: `test` returns stdout(), so a `--junitxml` written inside the transient container was never retrievable. Dropped the flag and the "one JUnit report" claims rather than advertise an artifact CI can't consume (exposing a report is a deliberate follow-up). - Reporting accuracy: the driver said "discovering in ..." for any plugin with an importable module regardless of whether it ships tests. Reworded to "handing N package(s) to pytest for collection"; docs/docstrings no longer claim a test-free plugin is "reported and skipped" (pytest just collects zero). - CLI: build.py module docstring no longer advertises the removed `plugin-regression` command; `_normalize` -> public `normalize_dist`. Verified: ruff/format/mypy/full pre-commit pass; 246 pytest pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
What & why
Makes
platform testverify a build cell end-to-end: it runs edx-platform'sown test suite AND the installed plugins' own suites in one run, under the
deployment's derived settings + plugin matrix — so a plugin bump, or a new
edx-platform commit, that breaks either surfaces in the same place.
Implements the "Plugin regression runner" task (project: lehrer CI verification
pipeline). Rebased onto
mainafter #110 merged.Design (updated per review direction)
Originally this was a separate
plugin_regressiontarget. Per direction, theplugin suites are now folded into
platform testrather than living in aseparate command — one image build, one pytest invocation, one JUnit report.
platform testgains--include-plugins(default true) and--install-test-extras(default true). It derives the plugin distributionsfrom the same requirement files
install_depsused, installs each maintainedol-*plugin's pinned[tests]extra (a verified safe no-op until the plugindefines one), and runs one pytest over the edx-platform paths plus the
installed plugin packages appended via
--pyargs(resolved at runtime fromimportlib.metadata— no hand-maintained map).--no-include-pluginsrunsthe edx-platform suite alone.
and the monorepo has no per-package tags, so "tests at the pinned version"
isn't obtainable. A plugin that ships no tests is reported and skipped, so
this stays green today and runs real plugin suites the moment one publishes a
[tests]extra (tracked in open-edx-plugins#830).plugin_regression.py→plugin_tests.py: keepsmaintained_test_extra_specs;the plugin-only driver becomes
combined_pytest_script(edx paths + pluginsin one run). Verified
--pyargsonly reinterprets non-path args, sofilesystem paths and import names coexist in one
pytest.plugin_regressionDagger function andlehrer build plugin-regressioncommand are removed.CLI facade (the "usable facade" ask)
lehrer build --helpgroups into Build / Verify / Utilities; the fourcell-scoped platform commands share one forwarding helper +
CellArg. Nocommand names changed (
--cell+ positional both work).Verification
lehrer-core-boundary) —pass. 246 pytest pass (
test_plugin_tests.py,test_build.py).dagger call platform test --helpshows--include-plugins/--install-test-extras;platform plugin-regressionis gone.discovery is a no-op-green today by design until plugins ship
[tests].Follow-ups
[tests]extras.platform test --include-pluginsinto the canary/matrix once real pluginsuites exist (deferred to avoid an empty gate).
🤖 Generated with Claude Code