Split manager behavior from environment lifecycle tests

[nvsekkin]

Description

Split simulator-free manager behavior from manager-based environment lifecycle coverage:

• keep real ManagerBasedEnv initialization, stepping, observation-history wiring, and recorder close/export integration tests
• move ObservationManager.compute(update_history=...) and RecorderManager registration, buffering, HDF5 creation, and direct-close behavior into focused unit tests
• reuse the shared core-only empty environment factories from Add kitless core environment test config factories #6557

This is PR5 of the focused replacement series for #6526.

Type of change

• Test refactor (no production behavior change)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Summary

This PR splits simulator-free manager behavior verification from manager-based environment lifecycle coverage, moving ObservationManager.compute(update_history=...) and RecorderManager unit-level tests into dedicated kitless files while keeping real-environment stepping and recorder close/export in focused integration tests.

• test_observation_manager_history.py (new): simulator-free unit test verifying that compute() only advances the history buffer when update_history=True, using a DummyEnv double.
• test_recorder_manager.py (refactored): demoted from integration to unit test by removing AppLauncher/SimulationContext; introduces DummyEnv, uses test_devices parametrization, and adds explicit close() calls to all tests.
• test_recorder_manager_integration.py (new): integration test verifying that ManagerBasedEnv.close() flushes buffered recorder data to HDF5; contains a double-close bug in its try/finally structure.
• test_manager_based_rl_env_ui.py (refactored): parametrizes the UI-window test over both has_gui values and properly restores the process-global setting.

Confidence Score: 4/5

The split is structurally clean and the new unit tests are well-isolated. One test in the new integration file calls env.close() twice on every run, which should be fixed before merge.

Five of the six changed files are straightforward: shared factory adoption, boilerplate removal, and two new well-structured test files. The new integration test in test_recorder_manager_integration.py closes the environment inside the try block as the operation under test, then closes it again unconditionally in finally. ManagerBasedEnv.close() flushes the recorder and tears down the simulation context; calling it a second time can raise or silently misbehave. All other changes are test-only refactors with no production impact.

source/isaaclab/test/managers/test_recorder_manager_integration.py — the try/finally structure always double-closes the environment.

Important Files Changed

Filename	Overview
source/isaaclab/test/managers/test_recorder_manager_integration.py	New integration test for recorder close/export; double env.close() bug in the try/finally structure.
source/isaaclab/test/managers/test_observation_manager_history.py	New simulator-free unit test for update_history semantics; clean DummyEnv double, correct assertions on buffer and current_length.
source/isaaclab/test/managers/test_recorder_manager.py	Demoted from integration to unit test; SimulationContext removed, DummyEnv introduced, test_devices parametrization added, explicit close() calls added to all tests.
source/isaaclab/test/envs/test_manager_based_env.py	Consolidated env-config boilerplate via shared factory; deterministic dummy_observation replaces the lambda; history test simplified and now just covers the step-side of the contract (non-step side moved to unit test).
source/isaaclab/test/envs/test_manager_based_rl_env_ui.py	Parametrized over both has_gui=False and has_gui=True; properly saves and restores the process-global setting in finally; env guarded with None before close.
source/isaaclab/changelog.d/core-test-manager-lifecycle.skip	Changelog skip fragment for this test-only refactor.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[test_manager_based_env.py\nintegration] -->|uses| F[make_empty_manager_based_env_cfg\nshared factory]
    B[test_manager_based_rl_env_ui.py\nintegration] -->|uses| G[make_empty_manager_based_rl_env_cfg\nshared factory]
    C[test_observation_manager_history.py\nunit - NEW] -->|DummyEnv double| H[ObservationManager\nupdate_history semantics]
    D[test_recorder_manager.py\nunit - refactored] -->|DummyEnv double| I[RecorderManager\nregistration + buffering + HDF5]
    E[test_recorder_manager_integration.py\nintegration - NEW] -->|uses| F
    E -->|ManagerBasedEnv| J[env.close exports\nbuffered recorder data]
    A -->|ManagerBasedEnv| K[step advances\nobs history]
    B -->|ManagerBasedRLEnv| L[has_gui gate controls\nwindow construction]

    style C fill:#d4f5d4
    style E fill:#d4f5d4
    style J fill:#ffe0e0

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[test_manager_based_env.py\nintegration] -->|uses| F[make_empty_manager_based_env_cfg\nshared factory]
    B[test_manager_based_rl_env_ui.py\nintegration] -->|uses| G[make_empty_manager_based_rl_env_cfg\nshared factory]
    C[test_observation_manager_history.py\nunit - NEW] -->|DummyEnv double| H[ObservationManager\nupdate_history semantics]
    D[test_recorder_manager.py\nunit - refactored] -->|DummyEnv double| I[RecorderManager\nregistration + buffering + HDF5]
    E[test_recorder_manager_integration.py\nintegration - NEW] -->|uses| F
    E -->|ManagerBasedEnv| J[env.close exports\nbuffered recorder data]
    A -->|ManagerBasedEnv| K[step advances\nobs history]
    B -->|ManagerBasedRLEnv| L[has_gui gate controls\nwindow construction]

    style C fill:#d4f5d4
    style E fill:#d4f5d4
    style J fill:#ffe0e0

Loading

_{Reviews (1): Last reviewed commit: "test: Split recorder manager coverage" | Re-trigger Greptile}