Skip to content

TR-20810: References are added incorrectly after closing test runs - #456

Open
acuanico-tr-galt wants to merge 12 commits into
release/1.15.3from
bugfix/TR-20810
Open

TR-20810: References are added incorrectly after closing test runs#456
acuanico-tr-galt wants to merge 12 commits into
release/1.15.3from
bugfix/TR-20810

Conversation

@acuanico-tr-galt

Copy link
Copy Markdown
Collaborator

Solution description

There is an issue where references are not correctly appended to runs when using --close-run option with --test-run-ref.

Changes

Defer the close_run operation when --test-run-ref is present, allowing references to be attached first.

Potential impacts

None

Steps to test

Parse a sample junit file and use --test-run-ref and --close-run together. There should be no error and references are added to the run correctly.

PR Tasks

  • PR reference added to issue
  • README updated
  • Unit tests added/updated

@gurock gurock deleted a comment from github-actions Bot Aug 3, 2026
Comment thread trcli/api/results_uploader.py
Comment thread trcli/commands/cmd_parse_junit.py
Comment thread trcli/commands/cmd_parse_junit.py
Comment thread trcli/constants.py
Comment thread trcli/api/variables_handler.py
Comment thread trcli/commands/cmd_datasets.py
Comment thread trcli/commands/cmd_variables.py
Comment thread trcli/commands/cmd_variables.py
Comment thread tests/test_cmd_datasets.py
@gurock gurock deleted a comment from github-actions Bot Aug 4, 2026
…interference and multisuite close-run regression
@gurock gurock deleted a comment from github-actions Bot Aug 4, 2026
@acuanico-tr-galt
acuanico-tr-galt requested a review from KayZ11 August 4, 2026 12:09
Comment thread trcli/commands/cmd_parse_junit.py Outdated
# Normal mode: process each suite separately
# Defer close_run if test_run_ref is provided to attach references first
defer_close = environment.test_run_ref is not None
run_ids = [] # Track all run IDs created/used

@KayZ11 KayZ11 Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_ids is scoped inside the else: branch but read outside it → crash in multisuite mode
When special_parser == "multisuite" the if branch at line 100 runs and line 112 never executes, so line 130 dereferences an unbound local. The old code was safe because it used run_id, which is initialised to None at line 96 before the branch.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've now Hoisted run_ids = [] to line 97 (top-level scope) so no more crashes in multisuite mode.

Comment thread trcli/commands/cmd_parse_junit.py Outdated
@@ -107,19 +107,35 @@ def cli(environment: Environment, context: click.Context, *args, **kwargs):
run_id = multisuite_uploader.last_plan_id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_id is now written at lines 96, 107 and 122–123 and never read again. I checked every occurrence in the file — after line 123 only run_ids / current_run_id are used.

So even once the scope bug is fixed by hoisting run_ids = [], multisuite mode would take the if environment.test_run_ref and run_ids: branch with an empty list and silently skip reference attachment entirely. Before this commit, _handle_test_run_references(environment, run_id) did attach refs to the plan ID. That behaviour is lost.

The # Keep first run_id for backward compatibility comment at line 121 is misleading — nothing consumes it. Either feed the multisuite plan ID into run_ids (run_ids = [multisuite_uploader.last_plan_id]) or drop run_id and handle the multisuite branch explicitly. Worth deciding deliberately, since it also settles the "does --close-run apply to multisuite plans?" question I raised earlier.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIxed to populate run_ids = [plan_id] in multisuite branch

Comment thread trcli/commands/cmd_parse_junit.py Outdated
# Handle test run references and closing for all runs
if environment.test_run_ref and run_ids:
# Attach references to all runs
for current_run_id in run_ids:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_handle_test_run_references ends with its own print(json.dumps(result, indent=2)) under if environment.json_output: (line 226). Looping it over N runs concatenates N standalone top-level JSON objects, which jq and json.loads both reject.

Slightly ironic given this same commit fixes JSON purity in the datasets/variables commands. If multi-run JSON output matters, accumulate the per-run results and emit one array after the loop.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this by accumulating results into array, print single valid JSON.

@gurock gurock deleted a comment from github-actions Bot Aug 5, 2026
@acuanico-tr-galt
acuanico-tr-galt requested a review from KayZ11 August 5, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants