mirror of
https://github.com/openai/codex.git
synced 2026-04-27 08:05:51 +00:00
tests(js_repl): remove node-related skip paths from js_repl tests (#12185)
## Summary Remove js_repl/node test-skip paths and make Node setup explicit in CI so js_repl tests always run instead of silently skipping. ## Why We had multiple “expediency” skip paths that let js_repl tests pass without actually exercising Node-backed behavior. This reduced CI signal and hid runtime/environment regressions. ## What changed ### CI - Added Node setup using `codex-rs/node-version.txt` in: - `.github/workflows/rust-ci.yml` - `.github/workflows/bazel.yml` - Added a Unix PATH copy step in Bazel workflow to expose the setup-node binary in common paths. ### js_repl test harness - Added explicit js_repl sandbox test configuration helpers in: - `codex-rs/core/src/tools/js_repl/mod.rs` - `codex-rs/core/src/tools/handlers/js_repl.rs` - Added Linux arg0 dispatch glue for js_repl tests so sandbox subprocess entrypoint behavior is correct under Linux test execution. ### Removed skip behavior - Deleted runtime guard function and early-return skips in js_repl tests (`can_run_js_repl_runtime_tests` and related per-test short-circuits). - Removed view_image integration test skip behavior: - dropped `skip_if_no_network!(Ok(()))` - removed “skip on Node missing/too old” branch after js_repl output inspection. ## Impact - js_repl/node tests now consistently execute and fail loudly when the environment is not correctly provisioned. - CI has stronger signal for js_repl regressions instead of false green from conditional skips. ## Testing - `cargo test -p codex-core` (locally) to validate js_repl unit/integration behavior with skips removed. - CI expected to surface any remaining environment/runtime gaps directly (rather than masking them). #### [git stack](https://github.com/magus/git-stack-cli) - ✅ `1` https://github.com/openai/codex/pull/12300 - ✅ `2` https://github.com/openai/codex/pull/12275 - ✅ `3` https://github.com/openai/codex/pull/12205 - ✅ `4` https://github.com/openai/codex/pull/12407 - ✅ `5` https://github.com/openai/codex/pull/12372 - 👉 `6` https://github.com/openai/codex/pull/12185 - ⏳ `7` https://github.com/openai/codex/pull/10673
This commit is contained in:
committed by
GitHub
parent
ddfa032eb8
commit
9501669a24
@@ -12,7 +12,6 @@ use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use which::which;
|
||||
use wiremock::MockServer;
|
||||
|
||||
fn custom_tool_output_text_and_success(
|
||||
@@ -55,25 +54,10 @@ async fn run_js_repl_turn(
|
||||
Ok(second_mock)
|
||||
}
|
||||
|
||||
fn can_run_js_repl_runtime_tests() -> bool {
|
||||
// In the base CI-stabilization commit, skip these integration tests only
|
||||
// when the js_repl Node runtime is not available. A later stacked commit
|
||||
// removes this automatic skip and requires them everywhere.
|
||||
if let Some(path) = std::env::var_os("CODEX_JS_REPL_NODE_PATH")
|
||||
&& std::path::Path::new(&path).exists()
|
||||
{
|
||||
return true;
|
||||
}
|
||||
which("node").is_ok()
|
||||
}
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn js_repl_persists_top_level_bindings_and_supports_tla() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if !can_run_js_repl_runtime_tests() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.features.enable(Feature::JsRepl);
|
||||
@@ -138,10 +122,6 @@ async fn js_repl_persists_top_level_bindings_and_supports_tla() -> Result<()> {
|
||||
async fn js_repl_can_invoke_builtin_tools() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if !can_run_js_repl_runtime_tests() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mock = run_js_repl_turn(
|
||||
&server,
|
||||
@@ -169,10 +149,6 @@ async fn js_repl_can_invoke_builtin_tools() -> Result<()> {
|
||||
async fn js_repl_tool_call_rejects_recursive_js_repl_invocation() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if !can_run_js_repl_runtime_tests() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mock = run_js_repl_turn(
|
||||
&server,
|
||||
@@ -214,10 +190,6 @@ try {
|
||||
async fn js_repl_does_not_expose_process_global() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if !can_run_js_repl_runtime_tests() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mock = run_js_repl_turn(
|
||||
&server,
|
||||
@@ -242,10 +214,6 @@ async fn js_repl_does_not_expose_process_global() -> Result<()> {
|
||||
async fn js_repl_blocks_sensitive_builtin_imports() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
if !can_run_js_repl_runtime_tests() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mock = run_js_repl_turn(
|
||||
&server,
|
||||
|
||||
Reference in New Issue
Block a user