Bugfix Issue 1331 UndefinedArrayKey execution_type EWarning - sebiboga/testlink-upgraded GitHub Wiki

Bugfix — Issue #1331: testcase::create()/update() — E_WARNING 'Undefined array key "execution_type"' when steps omit execution_type

Problem

Calling the TestLink testcase manager with step entries that lack an execution_type key raises 1x per step E_WARNING Undefined array key "execution_type", logged to the events table (log_level 2, source=GUI, activity=PHP, description points to lib/functions/testcase.class.php Line 6420). The HTTP response stays 200 and the operation succeeds — the symptom is only visible in the Event Viewer / events table.

Reproduce at HEAD (measured on the fresh-import CI box, PHP 8.3 / MySQL 11.4): create a test project + suite fixture, then call

$tcaseMgr->update($tc_id, $tcversion_id, 'name', 'summary', 'precond',
    ['step_number' => 1, 'actions' => 'a', 'expected_results' => 'e'](/sebiboga/testlink-upgraded/wiki/'step_number'-=>-1,-'actions'-=>-'a',-'expected_results'-=>-'e'), 1);

with steps that omit execution_type → after testcase::update() one new events row appears:

E_WARNING\nUndefined array key "execution_type" - in .../lib/functions/testcase.class.php - Line 6420

(4 steps ⇒ 4 warnings, as reported.) The XML-RPC updateTestCase path is hit identically because it forwards caller-supplied steps straight into update_tcversion_steps().

Root Cause

  • lib/functions/testcase.class.php:6417-6420 (update_tcversion_steps()) reads $steps[$idx]['execution_type'] unconditionally and passes it positionally to create_step($tcversion_id, $step_number, $actions, $expected_results, $execution_type).
  • create_step() (testcase.class.php:5779-5781) declares $execution_type = TESTCASE_EXECUTION_TYPE_MANUAL as its signature default, but the default can never fire: the caller evaluates $steps[$idx]['execution_type'] before the call, and PHP 8 emits the E_WARNING when the key is absent.
  • watchPHPErrors() (lib/functions/logger.class.php:1407-1446) forwards the E_WARNING to logWarningEvent() → tLog(..., "WARNING", ...) → events table (log_level 2).
  • The sibling entry point already handles this correctly: createVersion() guards with isset($item->steps[$jdx]['execution_type']) ? ... : TESTCASE_EXECUTION_TYPE_MANUAL (testcase.class.php:816-818) — update_tcversion_steps() never got the same guard.

Why it breaks now: any fixture/API caller (or XML-RPC client) that passes step arrays omitting execution_type to testcase::update() or XML-RPC updateTestCase. The modern tcEdit BFF always injects execution_type, so the UI is unaffected — consistent with the report.

Blast radius: both call sites of update_tcversion_steps(): testcase.class.php:1496 (inside testcase::update(), lib/api/xmlrpc/v1/xmlrpc.class.php:7321 (updateTestCase)). No other callers.

Fix

Committed on branch fix/issue-1331, commit 3fe9d44cb:

$this->create_step($tcversion_id,$steps[$idx]['step_number'],
                   $steps[$idx]['actions'],
                   $steps[$idx]['expected_results'],
                   $steps[$idx]['execution_type'] ?? TESTCASE_EXECUTION_TYPE_MANUAL);

The empty-coalescing operator makes the missing-key case default the step to TESTCASE_EXECUTION_TYPE_MANUAL — exactly the value create_step()'s signature default intended. It mirrors the createVersion() guard and reuses the same defaulting idiom already present in create_step() (testcase.class.php:5788-5793, there expressed as an isset(...) ? ... : ... ternary).

Rejected alternatives: (a) guarding with an explicit isset(...) ? ... : ... ternary — functionally identical but more verbose than the ?? operator already used in the file; (b) refactoring update_tcversion_steps() to call create_step() with the array signature — a behavior-neutral but larger change out of scope for a one-line log-noise bug.

Files Changed

  • lib/functions/testcase.class.php — line 6420: $steps[$idx]['execution_type'] → $steps[$idx]['execution_type'] ?? TESTCASE_EXECUTION_TYPE_MANUAL.
  • tmp/TLU_Test_Cases.md — regression suite "Regression — Issue #1331", 7/7 PASS.
  • Repro/regression tooling: tmp/repro_1331.php, tmp/repro_1331_update.php, tmp/regr_1331.php (gitignored local fixtures).

Verification

All checks on branch fix/issue-1331, PHP 8.3, MySQL fresh-import schema:

  • Pre-fix testcase::update() with a step missing execution_type → 1x E_WARNING ... testcase.class.php - Line 6420 in events (log_level 2).
  • Post-fix the same call → update succeeds, 0 E_WARNING rows; the step row lands in tcsteps with execution_type=1 (MANUAL default at DB level).
  • Explicit-value regression: update with execution_type=2 → step kept execution_type=2 (no default-clobber).
  • testcase::create() (steps without the key) already guarded by createVersion() → 0 warnings, create succeeds.
  • Import-parity: fresh schema re-import + both repro scripts re-run → 0 E_WARNING. SELECT COUNT(*) FROM events WHERE description LIKE 'E\_WARNING%' = 0.
  • Event Viewer after the whole pass → only INFO/AUDIT entries (audit_testproject_created etc., log_level 16); no log_level 2/3 rows. Browser check (headless Chrome, admin session, eventviewer.html): WARNING=0, ERROR=0, DEBUG=0, INFO=0, L18N=0; only 4 AUDIT rows (login + the BUG1331 project create/delete fixture audit): ![Event Viewer clean after fix](issue-1331-eventviewer-clean.png)

Regression suite: tmp/TLU_Test_Cases.md — "Regression — Issue #1331", 7/7 PASS.

Address: Fixes #1331 (verified + pushed on fix/issue-1331).