Consolidate Rogue regression jobs into a single Rogue Regression Tests runner - #1480
Merged
Merged
Conversation
Merge the SimLink Rogue Contract and ADC DDR Rogue Tests jobs into one Rogue Regression Tests job, and rename Regression Tests to Non-rogue Regression Tests so the boundary between the two runners is explicit. The Rogue/non-Rogue split is a packaging boundary, not a testing one. Rogue ships only through the tidair-tag conda channel and is absent from pip_requirements.txt, so Rogue coverage needs Miniforge and a conda login shell, while the main regression job deliberately builds and simulates on apt mcode GHDL. Rogue coverage also has to stay a distinct required suite because every Rogue test skips silently when Rogue is missing, so inside the non-rogue job it would be an invisible no-op. The two Rogue jobs had no such justification for being separate. Their conda environments were identical apart from the name field, both ran serially, and each paid the Miniforge provisioning cost on its own. Per-suite differences are preserved as step-scoped settings. PYTHONPATH stays on the ADC DDR step, where it forces the in-repo PyRogue model, and SIMLINK_ROGUE_PYTHON stays on the SimLink step. The ruckus import is hoisted to job level because the SimLink contract needs it and it is inert for the pure-PyRogue suites. A new Verify pinned Rogue step fails before any suite runs, so a broken conda activation cannot quietly degrade the job into skips. Replace the duplicate tests/simlink/rogue/conda.yml with the repository-root conda-rogue.yml, and update the docs that referenced the old environment file and the old job names. Which tests execute is unchanged, so a green pipeline exercises the restructure alone.
ruck314
marked this pull request as ready for review
August 28, 2026 18:57
bengineerd
approved these changes
Sep 9, 2026
cocotb 2.1.0 moved responsibility for building GPI_USERS onto the test runner. The VPI/VHPI shims now refuse to start the embedded interpreter without it and abort elaboration with gpi_load_users No GPI_USERS specified, exiting... leaving a zero-length results.xml that pytest reports as an unrelated xml.etree ParseError. cocotb's own runner sets the variable in _set_env_common(); cocotb-test does not, so every suite in this repo broke on the first push after that release with no source change of ours involved. Export GPI_USERS from a new tests/conftest.py. pytest loads it for every invocation, since pytest.ini fixes rootdir at the repo root and all CI and tests/simlink/run-*.sh targets live under tests/. It sets os.environ rather than passing extra_env for two reasons: cocotb-test copies os.environ over extra_env after applying it, so os.environ is the layer that reaches the simulator, and run_surf_vhdl_test() folds extra_env keys into the sim_build directory name, so a path-valued variable there would rename every parameterized build directory. Raise the floor to cocotb>=2.1 in both manifests. The shim needs cocotb_tools.config.pygpi_entry_point, which 2.0.x does not provide, so the conftest probes for it and names the requirement instead of failing with a bare AttributeError. Verified against cocotb 2.1.0: 427 passed, 8 skipped across tests/axi, tests/base, tests/dsp and tests/simlink.
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
2 participants
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.
Description
Consolidate all Rogue-dependent CI coverage into one
Rogue Regression Testsjob, and renameRegression TeststoNon-rogue Regression Testsso the boundary between the two runners is self-documenting. The previously separateSimLink Rogue ContractandADC DDR Rogue Testsjobs are now steps of the single Rogue job, which removes a duplicated Miniforge provisioning cost and gives future real-Rogue suites an obvious home. The duplicatetests/simlink/rogue/conda.ymlis replaced by the repository-rootconda-rogue.yml. Which tests execute is unchanged.Details
Why Rogue stays off the main regression runner. The split is a packaging boundary, not a testing one:
tidair-tagconda channel and is absent frompip_requirements.txt. The non-rogue job is apt + pip +actions/setup-python; Rogue coverage needs Miniforge andshell: bash -el {0}. Folding Rogue in would move the whole cocotb/GHDL regression onto a conda interpreter, whereas full runs deliberately build and simulate on apt mcode GHDL.pytest.importorskipin the ADC DDR pair,_check_rogue_python()in the SimLink contracts), so inside the non-rogue job they would be invisible no-ops.tests/simlink/README.mdalready requires real-Rogue coverage to remain a distinct required suite.Why the two Rogue jobs did not need to be separate. Their conda environments were identical apart from the
name:field, both ran-n 0, both re-verifiedrogue.Version.current(), and each paid Miniforge provisioning independently.Merge correctness. Per-suite differences are preserved as step-scoped settings rather than hoisted:
PYTHONPATH="$PWD/python"surfSIMLINK_ROGUE_PYTHONmake MODULES="$PWD" importtimeout-minutesA new
Verify pinned Roguestep runs before any suite so a broken conda activation fails loudly instead of degrading the job into skips. The workflow header now records the rationale for the split so it does not have to be re-derived.Verification. YAML parses and the job graph resolves;
conda-rogue.ymlname:matchesactivate-environment; all pytest targets exist; no danglingsimlink_rogue/adc_ddr_rogue/surf-simlink-roguereferences remain;flake8 --count python/ scripts/ tests/reports 0;pytest tests/commonpasses 89 tests; the path selector returnsFORCE_FULLfor this change set, so this push self-tests at full scope. The Rogue suites themselves could not be run locally (no conda available on the dev host) and are exercised by this PR's own CI run.Maintainer action required. GitHub matches required status checks by job display name, so branch protection on
mainandpre-releasemust be updated or PRs will block on checks that can no longer report:Regression TestsNon-rogue Regression TestsSimLink Rogue ContractRogue Regression TestsADC DDR Rogue TestsRogue Regression TestsWiring the existing but unwired
test_RogueStreamRogue.pyandtest_RogueSideBandRogue.pycontracts into the new job remains the tracked follow-up already noted insimlink/docs/rogue-clients.md.