Skip to content

aunit: enable FUGR and FUNC AUnit runs#180

Merged
jfilak merged 1 commit into
jfilak:masterfrom
filak-sap:aunit_fugr_func
Jun 26, 2026
Merged

aunit: enable FUGR and FUNC AUnit runs#180
jfilak merged 1 commit into
jfilak:masterfrom
filak-sap:aunit_fugr_func

Conversation

@filak-sap

Copy link
Copy Markdown
Contributor

There is no fundamental reason to not have FUGR and FUNC enabled. One just needed to let me know it should work.

There is no fundamental reason to not have FUGR and FUNC enabled.
One just needed to let me know it should work.
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The AUnit CLI, request mapping, docs, and tests now accept function-group and function-module object types. function-module is mapped to FUNC, and the unit and CLI tests cover the updated XML and HTTP request flow.

Changes

AUnit object types

Layer / File(s) Summary
CLI surface
sap/cli/aunit.py, doc/commands/aunit.md
aunit now accepts function-group and function-module, and the run command documentation shows the expanded object-type list.
Function-module mapping
sap/adt/api/aunit.py, test/unit/test_sap_adt_api_aunit.py
function-module maps to FUNC, and the AUnit XML fixture and assertion are updated for that request shape.
CLI request flow tests
test/unit/test_sap_cli_aunit.py
New CLI tests cover explicit function-group and function-module runs, generated request bodies, and the no-test-classes response path.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: enabling FUGR and FUNC AUnit runs.
Description check ✅ Passed The description is on-topic and describes enabling FUGR and FUNC AUnit support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 12096ed and 4b993dd.

📒 Files selected for processing (5)
  • doc/commands/aunit.md
  • sap/adt/api/aunit.py
  • sap/cli/aunit.py
  • test/unit/test_sap_adt_api_aunit.py
  • test/unit/test_sap_cli_aunit.py

Comment thread doc/commands/aunit.md
Comment on lines 18 to 19
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 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.

Suggested change
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

Comment on lines +168 to +170
def test_function_group(self):
xml = Marshal().serialize(build_test_run([('zfunction', 'function-module')]))
self.assertEqual(xml, EXPECTED_XML_FUNC)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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.

Suggested change
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

@jfilak jfilak merged commit ce79be1 into jfilak:master Jun 26, 2026
3 checks passed
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