Force interval events by default

[nvsekkin]

Description

• Follow up PR to Speed up environment smoke tests and cover EventManager interval handlers #5713
• Update force_interval_events to be default on all test helpers
• If there is no interval mode, this function is a no-op. For all others, this helps test the interval critical path at each step() call.
• Currently the following files will benefit from this change: test_environments.py, test_environments_with_stage_in_memory.py, test_environments_newton.py, and test_factory_environments.py.
• 9 of the 12 newly-affected files are no-ops

Fixes:

• Forge dead_zone_thresholds fired ~3×/env pre-Speed up environment smoke tests and cover EventManager interval handlers #5713, dropped to 0×/env after Speed up environment smoke tests and cover EventManager interval handlers #5713's 100→20 step cut, restored to 20×/env by this PR.
• push_robot terms never fired even in the legacy 100-step budget. This PR is the first time they're exercised in CI.

Type of change

• Test 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 flips the default value of force_interval_events from False to True in both _run_environments and _check_random_actions in env_test_utils.py, so interval-mode event terms fire immediately during tests without callers having to opt in. The test_environments.py call site is simplified by removing the now-redundant explicit force_interval_events=True argument.

• env_test_utils.py: default changed to True in both utility functions; no logic changes otherwise.
• test_environments.py: call simplified from a multi-line keyword-argument form to a single-line call relying on the new default.
• changelog.d/esekkin-force-interval-events-default.skip: correctly marks the entry as a test-only change that skips changelog generation.

Confidence Score: 4/5

Safe to merge; the change is intentional and test-scoped, but worth confirming all affected callers are expected to run with forced interval events.

The core change is a one-line default flip in a test utility. The direct call site in test_environments.py is correctly simplified. The only concern is that 11+ other test files that rely on the old False default will now silently run with forced interval events without any code change on their side — if those callers were intentionally testing normal event timing, their coverage changes quietly.

source/isaaclab_tasks/test/env_test_utils.py — the default change implicitly affects all callers; the other test files that call these utilities without an explicit force_interval_events argument should be verified.

Important Files Changed

Filename	Overview
source/isaaclab_tasks/test/env_test_utils.py	Default of force_interval_events flipped to True in both _run_environments and _check_random_actions, silently changing the behavior of all callers that don't explicitly set this parameter.
source/isaaclab_tasks/test/test_environments.py	Call simplified by removing the now-redundant force_interval_events=True kwarg; functionally equivalent to the original.
source/isaaclab_tasks/changelog.d/esekkin-force-interval-events-default.skip	Changelog skip file appropriately marks this as a test-only change that does not need a public changelog entry.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Test file calls _run_environments / _check_random_actions] --> B{force_interval_events\npassed explicitly?}
    B -- Yes --> C[Use provided value]
    B -- No --> D["Use default (was False, now True)"]
    C --> E{force_interval_events == True?}
    D --> E
    E -- Yes --> F[_force_interval_events_to_fire_immediately\nsets interval_range_s to 1e-6]
    E -- No --> G[Interval events use original timing]
    F --> H[env.reset + env.step loop]
    G --> H

Loading

_{Reviews (1): Last reviewed commit: "Force interval events by default" | Re-trigger Greptile}

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR changes the default value of force_interval_events from False to True in two test utility functions (_run_environments and _check_random_actions). This is a test-only change that improves test coverage by ensuring interval-mode event terms fire on the first step() call during environment testing.

---

📋 Review

✅ What This Change Does

• env_test_utils.py: Flips the default of force_interval_events parameter from False to True in both:

  • _run_environments() (line 262)
  • _check_random_actions() (line 354)

• test_environments.py: Simplifies the test_environments test by removing the explicit force_interval_events=True argument (now redundant since it's the default)

• Changelog fragment: Correctly adds a .skip changelog entry documenting this as a test-only change

✅ Backward Compatibility Assessment

Aspect	Impact
Production code	✅ No impact - test utilities only
Existing tests calling these functions	⚠️ Behavioral change - tests relying on force_interval_events=False will now fire interval events by default
CI/CD	✅ Should improve test reliability

🔍 Analysis

Why this change makes sense:

1. The force_interval_events=True setting forces interval-mode event terms (like randomization events) to fire immediately on the first step(), ensuring they're exercised during testing
2. Previously, tests might skip exercising these events if the sampled time_left didn't trigger within the test's step count
3. This provides more thorough test coverage by default

Potential considerations:

• Any external or downstream tests using _run_environments() or _check_random_actions() without explicitly setting force_interval_events will now get the new default behavior
• This is generally beneficial for test thoroughness, but could expose previously hidden issues in event handlers

⚠️ CI Status

Note: "Installation Tests (uv)" is currently failing. Please verify this is unrelated to the changes in this PR.

---

📊 Verdict

Category	Assessment
Code Quality	✅ Clean, minimal change
Test Coverage Impact	✅ Positive - more thorough by default
Breaking Change Risk	⚠️ Low - test utils only, could affect downstream test behavior
Documentation	✅ Changelog fragment present

Overall: This is a sensible improvement to test defaults. The change ensures interval events are exercised in tests by default, improving coverage. The simplification of test_environments.py is a nice cleanup.

---

_{🤖 Generated by Isaac Lab Review Bot | About}