aunit: enable FUGR and FUNC AUnit runs#180
Conversation
There is no fundamental reason to not have FUGR and FUNC enabled. One just needed to let me know it should work.
📝 WalkthroughWalkthroughThe AUnit CLI, request mapping, docs, and tests now accept ChangesAUnit object types
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/commands/aunit.md`:
- Around line 18-19: The documentation text in aunit.md has a typo and awkward
phrasing: correct “erroed” to “errored” and tighten the sentence for clarity.
Update the wording around the exit code behavior in the aunit command docs so it
reads smoothly and accurately, keeping the meaning aligned with the existing
description of failed tests and the _unit_ result.
In `@test/unit/test_sap_adt_api_aunit.py`:
- Around line 168-170: The test method name is duplicated, which shadows the
earlier function-group coverage and prevents the FUGR assertion from running.
Update the second test in test_sap_adt_api_aunit.py by renaming the method
currently defined as test_function_group so it clearly represents the
function-module case, and keep the original function-group test name reserved
for the EXPECTED_XML_FUGR assertion. Use the existing symbols Marshal,
build_test_run, EXPECTED_XML_FUNC, and EXPECTED_XML_FUGR to ensure both test
cases remain distinct and executed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fbd553fa-e64c-4e8d-98af-3657bcbfe04f
📒 Files selected for processing (5)
doc/commands/aunit.mdsap/adt/api/aunit.pysap/cli/aunit.pytest/unit/test_sap_adt_api_aunit.pytest/unit/test_sap_cli_aunit.py
| The exit code will be determined based on test results where exit code is the | ||
| number of failed and erroed tests if _unit_ included in the result. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Typo: "erroed" → "errored".
Also consider tightening the phrasing for clarity.
📝 Proposed fix
-The exit code will be determined based on test results where exit code is the
-number of failed and erroed tests if _unit_ included in the result.
+The exit code will be determined based on test results where the exit code is the
+number of failed and errored tests if _unit_ is included in the result.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The exit code will be determined based on test results where exit code is the | |
| number of failed and erroed tests if _unit_ included in the result. | |
| The exit code will be determined based on test results where the exit code is the | |
| number of failed and errored tests if _unit_ is included in the result. |
🧰 Tools
🪛 LanguageTool
[grammar] ~19-~19: Ensure spelling is correct
Context: ...e exit code is the number of failed and erroed tests if unit included in the result....
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@doc/commands/aunit.md` around lines 18 - 19, The documentation text in
aunit.md has a typo and awkward phrasing: correct “erroed” to “errored” and
tighten the sentence for clarity. Update the wording around the exit code
behavior in the aunit command docs so it reads smoothly and accurately, keeping
the meaning aligned with the existing description of failed tests and the _unit_
result.
Source: Linters/SAST tools
| def test_function_group(self): | ||
| xml = Marshal().serialize(build_test_run([('zfunction', 'function-module')])) | ||
| self.assertEqual(xml, EXPECTED_XML_FUNC) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Duplicate test name silently drops FUGR coverage.
test_function_group is redefined here (line 168), shadowing the original function-group/EXPECTED_XML_FUGR test at line 164. Python keeps only the last definition, so the FUGR assertion never runs and EXPECTED_XML_FUGR is effectively unused. Rename this method to reflect the function-module case.
🐛 Proposed fix
- def test_function_group(self):
+ def test_function_module(self):
xml = Marshal().serialize(build_test_run([('zfunction', 'function-module')]))
self.assertEqual(xml, EXPECTED_XML_FUNC)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_function_group(self): | |
| xml = Marshal().serialize(build_test_run([('zfunction', 'function-module')])) | |
| self.assertEqual(xml, EXPECTED_XML_FUNC) | |
| def test_function_module(self): | |
| xml = Marshal().serialize(build_test_run([('zfunction', 'function-module')])) | |
| self.assertEqual(xml, EXPECTED_XML_FUNC) |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 168-168: redefinition of unused 'test_function_group' from line 164
(F811)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/unit/test_sap_adt_api_aunit.py` around lines 168 - 170, The test method
name is duplicated, which shadows the earlier function-group coverage and
prevents the FUGR assertion from running. Update the second test in
test_sap_adt_api_aunit.py by renaming the method currently defined as
test_function_group so it clearly represents the function-module case, and keep
the original function-group test name reserved for the EXPECTED_XML_FUGR
assertion. Use the existing symbols Marshal, build_test_run, EXPECTED_XML_FUNC,
and EXPECTED_XML_FUGR to ensure both test cases remain distinct and executed.
Source: Linters/SAST tools
There is no fundamental reason to not have FUGR and FUNC enabled. One just needed to let me know it should work.