Skip to content

Integration tests pass on compile errors due to repl_eval_with_variables_value error trap #618

Description

@jsavin

Integration tests pass on compile errors due to repl_eval_with_variables_value error trap

Surfaced during #586 / PR #617 investigation. Filing as a separate issue because the surface is broader than just parser fixes.

Symptom

Many integration tests in tests/integration/test_cases/*.yaml pass on develop only because the test script body fails to compile, and repl_eval_with_variables_value (via langruntraperror) traps the compile error and returns success=true with value="true". The yaml assertions are expected_success: true and expected_result: "true", which trivially pass regardless of what the script actually does.

Repro on develop

$ cat > /tmp/probe.json <<'EOF'
{"op":"script/eval","id":1,"params":{"expression":"local(x = 1);\n\nlocal(y = 2);\nreturn x + y"}}
EOF
$ ./frontier-cli/frontier-cli --protocol --skip-startup --allow-mutate --system-root tests/tmp/results/db/Frontier.root < /tmp/probe.json
[lang-ERROR] langcallbacks.c:249: line 1: Can't compile this script because is an illegal character.
[startup-ERROR] script_portable.c:160: headless lang error: Can't compile this script because is an illegal character.
{"id":1,"result":{"value":"true","type":"boolean"},"success":true}

Note: success=true and value="true" even though the script failed to compile.

Affected scope

PR #617 (#586 parser fix) revealed at least 85 tests across 8 yaml files that pass on develop only because of this behavior:

  • db_verbs.yaml (1)
  • menu_list_describe.yaml (3)
  • menu_data_verbs.yaml (4)
  • migration_externals.yaml (5)
  • op_verbs.yaml (4)
  • tcp_client_verbs.yaml (11)
  • tcp_phase2_verbs.yaml (9)
  • tcp_server_verbs.yaml (21)
  • plus mainresponder, webserver, inetd

Each of these fails when the parser is fixed and the script is allowed to compile and run for real.

Root cause

frontier-cli/repl_variables.c (repl_eval_with_variables_value, line ~417) calls langruntraperror which:

  1. Compiles the script.
  2. If compile fails, the error is logged but trapped.
  3. The function returns true with the last-evaluated value (which for a wrapped with system.temp.FrontierREPL.variables { ... } block defaults to boolean true).
  4. Protocol-mode script/eval (op_handler.c) maps that to success: true, result: {value: "true", type: "boolean"}.

The REPL UX rationale for trap-and-return is sound (a typo in the REPL shouldn't crash the loop), but it makes integration tests silently pass even when the script never executed.

Recommended paths

A. Distinguish compile errors in protocol mode. Have script/eval return success: false, error: "compile failed: ..." when langruntraperror traps a parse error. The REPL keeps its trap-and-continue behavior unchanged. This is the minimal, targeted fix.

B. Migrate tests to behavioral assertions. Many of the failing tests would benefit from expected_result that exercises actual script output, not the wrapper's trivially-true outcome. Some tests should also use result_type: string or check specific intermediate values.

C. Add a yaml-level option expect_compile: true that asserts the script compiled cleanly (separate from expected_success). Defaults to true; tests that intentionally exercise compile errors can opt out.

I suggest A + C as the path: protocol-mode surfaces compile failures distinctly, and the yaml runner enforces "if you didn't set expect_compile: false, the script must compile". B follows naturally from A.

Blocks

Labels

bug, priority/p1, scope/medium, test-infrastructure

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions