trunner: add formatting to subresult message with newlines

[adamdebek]

JIRA: CI-519

Description

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

• Already covered by automatic testing.
• New test added: (add PR link here).
• Tested by hand on: (list targets here).

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing linter checks and tests passed.
• My changes generate no new compilation warnings for any of the targets.

Special treatment

• This PR needs additional PRs to work (list the PRs, preferably in merge-order).
• I will merge this PR by myself when appropriate.

[github-actions]

Unit Test Results

7 958 tests  ±0   7 416 ✅ ±0   40m 9s ⏱️ -30s
  470 suites ±0     542 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit faf8d14. ± Comparison against base commit 1c676a4.

♻️ This comment has been updated with latest results.

[mateusz-bloch]

Overall, LGTM

I'm just thinking if we should go with a fixed spacing , it might look better, in my opinion. What do you think @damianloew ?

[nalajcie]

Why do we need this?

IMHO no offset is OK (as subresults are indented, so everything with "negative indent" is clearly distinguishable)?

Using lstrip here is invalid, any output which aligns message by hand will be distorted (eg. gcc output highlighting the error, python traceback, etc.)

[adamdebek]

I agree with removal of lstrip()

Example output with this change:

Output without this change:

[nalajcie]

Thanks for the visual example. Some notes:

• the trailing newline in every msg seems like a problem in that particular harness
• I'm a little bit afraid about aligning it as the (sub)test names might be long - eg. mbedTLS subtests can have len up to 70 chars - so aligning the output might actually decrease the readability in that case (might break long messages)?

If the above is a non-issue, we can merge it.

[adamdebek]

Yes, trailing new lines in 1 line message is from harness, but this harness took into account that pr and splitlines() trim them.

Maybe when (subtest name + longest fail message from msg lines) < some_edge_value.
Example abstract output:
\t very-long-testnameeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee: FAIL
\t\t message line 1
\t\t message line 2
...
\t\t message line n

[nalajcie]

Yes, trailing new lines in 1 line message is from harness, but this harness took into account that pr and splitlines() trim them.

Nah, this PR handles only writing to screen case, it will still look ugly in XML output. Please correct the msg on harness side if possible.

Maybe when (subtest name + longest fail message from msg lines) < some_edge_value. Example abstract output: \t very-long-testnameeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee: FAIL \t\t message line 1 \t\t message line 2 ... \t\t message line n

IMHO we should always present the result in the same way - otherwise the user might get confused.

I'm ok with custom per-test indentation if we acknowledge the potential drawbacks. I also don't have a problem with not-indenting multiline messages (as it's done for normal test results). Feel free to choose the solution you think works best.

[adamdebek]

I think it is still clear to the user when FAIL status and some indented text below is presented and IMHO it is the most readable