fix: junit report test

[Mohamed-el-hedi-dridi]

📚 Description

Fix JUnit XML report generation where all test results appeared as passing (green) even when some tests were failing. The root cause was that the JUnit XML format was using a status attribute on elements, which is not part of the standard JUnit XML schema and is therefore ignored by most CI/reporting tools. Standard-compliant parsers (e.g. GitLab CI) determine test outcomes based on the presence of child elements (, , ) inside each — not attributes.

🔖 Changes

• src/reports.sh: Replaced the non-standard status attribute on elements with proper JUnit XML child elements:
  Failed tests now emit a child element
  Skipped tests now emit a child element
  Incomplete tests now emit a child element
• Removed non-standard attributes (status, assertions, passed, incomplete, snapshot) from the and elements to align with the standard JUnit XML schema
• Added errors="0" attribute to for better compatibility with JUnit XML consumers

✅ To-do list

• I updated the CHANGELOG.md to reflect the new feature or fix
• I updated the documentation to reflect the changes

[Copilot]

Pull request overview

This PR fixes JUnit XML report generation so CI/reporting tools correctly mark failing/skipped tests by emitting standard JUnit child elements (<failure>, <skipped>) instead of relying on a non-standard status attribute.

Changes:

• Replace non-standard <testcase status="..."> with standard <failure> / <skipped> child elements.
• Remove several non-standard JUnit attributes and add errors="0" to the <testsuite> element.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Chemaclass]

Review

The bug described here is real and the fix direction is correct. Standard JUnit XML parsers (GitLab CI, Jenkins, CircleCI) determine test outcomes from child elements (<failure>, <skipped>) — not from the status attribute. So this is a genuine CI compatibility fix worth merging.

That said, there are a few blockers before it's ready.

---

🔴 Blockers

1. No tests for generate_junit_xml()

The JUnit XML generation has no test coverage at all in the suite, and this PR doesn't add any. Since this is a behaviour change in the output format, it needs at least a basic test to verify the XML structure for each status (passed, failed, skipped, incomplete).

Something along the lines of:

function test_junit_xml_marks_failed_tests_with_failure_element() {
  # set up a fake report with one failed test, generate XML, assert <failure> is present
}

Without tests, a future refactor could silently regress the exact thing this PR is fixing.

2. CHANGELOG.md not updated

This is a user-facing fix (CI pipelines that rely on JUnit output will start behaving differently). It needs a changelog entry. Your own checklist has this unchecked.

---

🟡 Important improvement: failure messages are not useful

The <failure> element currently emits a hardcoded string with no actual content:

<failure message="Test failed" type="AssertionError">Test failure details</failure>

This defeats much of the purpose of JUnit XML in CI — the failure detail is what developers read to understand what went wrong. I understand the data model (_BASHUNIT_REPORTS_TEST_* arrays) doesn't currently store failure output, which makes this a bigger change. But the fix as written trades one usability problem (tests always appear green) for another (tests appear red but you can't tell why).

A suggested path forward:

1. Add a _BASHUNIT_REPORTS_TEST_MESSAGES array alongside the existing ones
2. Pass the failure message through bashunit::reports::add_test_failed
3. Emit it inside <failure>

This doesn't have to block the merge if it's tracked as a follow-up issue, but it's worth acknowledging in the PR description.

---

🟢 What's good

• Correctly replaces status attribute with <failure>, <skipped>, <skipped message="..."/> child elements
• errors="0" on <testsuite> improves compatibility
• Removal of non-standard attributes (passed, assertions, incomplete, snapshot) aligns the output closer to the spec
• Handles incomplete mapped to <skipped message="Test incomplete"/> which is the standard-compliant way to represent it

[Mohamed-el-hedi-dridi]

Review

The bug described here is real and the fix direction is correct. Standard JUnit XML parsers (GitLab CI, Jenkins, CircleCI) determine test outcomes from child elements (<failure>, <skipped>) — not from the status attribute. So this is a genuine CI compatibility fix worth merging.

That said, there are a few blockers before it's ready.

🔴 Blockers

1. No tests for generate_junit_xml()

The JUnit XML generation has no test coverage at all in the suite, and this PR doesn't add any. Since this is a behaviour change in the output format, it needs at least a basic test to verify the XML structure for each status (passed, failed, skipped, incomplete).

Something along the lines of:

function test_junit_xml_marks_failed_tests_with_failure_element() {
  # set up a fake report with one failed test, generate XML, assert <failure> is present
}

Without tests, a future refactor could silently regress the exact thing this PR is fixing.

2. CHANGELOG.md not updated

This is a user-facing fix (CI pipelines that rely on JUnit output will start behaving differently). It needs a changelog entry. Your own checklist has this unchecked.

🟡 Important improvement: failure messages are not useful

The <failure> element currently emits a hardcoded string with no actual content:

<failure message="Test failed" type="AssertionError">Test failure details</failure>

This defeats much of the purpose of JUnit XML in CI — the failure detail is what developers read to understand what went wrong. I understand the data model (_BASHUNIT_REPORTS_TEST_* arrays) doesn't currently store failure output, which makes this a bigger change. But the fix as written trades one usability problem (tests always appear green) for another (tests appear red but you can't tell why).

A suggested path forward:

1. Add a _BASHUNIT_REPORTS_TEST_MESSAGES array alongside the existing ones
2. Pass the failure message through bashunit::reports::add_test_failed
3. Emit it inside <failure>

This doesn't have to block the merge if it's tracked as a follow-up issue, but it's worth acknowledging in the PR description.

🟢 What's good

• Correctly replaces status attribute with <failure>, <skipped>, <skipped message="..."/> child elements
• errors="0" on <testsuite> improves compatibility
• Removal of non-standard attributes (passed, assertions, incomplete, snapshot) aligns the output closer to the spec
• Handles incomplete mapped to <skipped message="Test incomplete"/> which is the standard-compliant way to represent it

done