Skip to content

[UX/UI] Add interactive checklist shell script modules for package management #530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

Tearran
Copy link
Member

@Tearran Tearran commented Apr 15, 2025

[generated summary]
This pull request introduces new reusable helper functions for package management and dynamic checklists in the tools/modules directory. The changes include adding new functions to handle interactive checklists for various package groups and updating module options to reflect these new capabilities.

New Helper Functions:

  • tools/modules/functions/interface_checklist.sh: Added a reusable helper function interface_checklist to display a checklist using whiptail, dialog, or read. This function prepares and displays checklist options based on the specified dialog tool and returns the selected items.
  • tools/modules/functions/interface_checklist.sh: Added a reusable helper function process_package_selection to process user-selected packages for installation or removal. This function uses the interface_checklist function to display the checklist and then installs or removes the selected packages accordingly.

Dynamic Package Management:

  • tools/modules/software/module_aptwizard.sh: Added _checklist_proftpd function to dynamically manage ProFTPD packages with an install/remove toggle. This function fetches ProFTPD-related packages, prepares checklist options, and processes user selections using process_package_selection.
  • tools/modules/software/module_aptwizard.sh: Added _checklist_browsers function to manage browser installation and removal (e.g., Firefox-ESR, Chromium). This function fetches browser package details, prepares checklist options, and processes user selections using process_package_selection.
  • tools/modules/software/module_aptwizard.sh: Added _checklist_editors function to manage editor installation and removal (e.g., nano, code, notepadqq). This function fetches editor package details, prepares checklist options, and processes user selections using process_package_selection.

UX/UI is the proposed changes. The so called aptwizard is only intended to show a practical application of the UI helper.

Copy link
Contributor

coderabbitai bot commented Apr 15, 2025

Walkthrough

This change adds a new interactive package management feature named "AptWizard" for Debian-based systems. It introduces a shell script module (module_aptwizard.sh) that provides checklist-driven interfaces for managing packages in categories such as ProFTPD, browsers, editors, and imaging editors. The module dynamically retrieves package information and installation status, allowing users to select packages to install or remove through a checklist UI. Supporting functions for displaying checklists and processing package selections are implemented in a separate script (interface_checklist.sh). Metadata describing authorship, feature details, and supported architectures is included for each component. A new submenu item for "AptWizard" is added to the software configuration JSON file, enabling access from the main software menu. Additionally, a test configuration file is added to verify that the module_aptwizard API help command runs without error. No existing entities are removed or altered.

Suggested labels

ready to merge


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c06e60 and 7175468.

📒 Files selected for processing (1)
  • tools/json/config.software.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/json/config.software.json

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the size/large PR with 250 lines or more label Apr 15, 2025
@Tearran
Copy link
Member Author

Tearran commented Apr 15, 2025

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Apr 15, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label Apr 15, 2025
@Tearran Tearran marked this pull request as ready for review April 15, 2025 07:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
tools/modules/functions/interface_checklist.sh (3)

1-10: Consider adding a shebang or shell directive for static analysis compliance.

While this file is likely intended to be sourced, adding a comment such as # shellcheck shell=bash at the top can help static analysis tools and clarify the intended shell.

🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


12-61: Robust, reusable checklist UI helper—well-structured and flexible.

The interface_checklist function is well-designed, supporting multiple UI backends and fallback. The use of a nameref for the options array is modern and efficient. Error handling for user cancellation is present.

Minor suggestions:

  • Consider trimming whitespace from user input in the "read" fallback to avoid accidental selection errors.
  • If the script is ever run with set -u, uninitialized variables (like selected_indexes) could cause issues; ensure defaults are set or check before use.

74-104: Effective package selection processor—clear logic and user feedback.

The process_package_selection function is clear and effective, leveraging the checklist UI and handling install/remove logic based on current package state. The use of array references is correct.

Suggestions:

  • Consider quoting $package in the dpkg -l | grep command to avoid issues with package names containing special characters.
  • If pkg_install or pkg_remove can fail, consider capturing and reporting errors for better UX.
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 77-77: checklist_options appears unused. Verify use (or export if used externally).

(SC2034)

tools/modules/software/module_aptwizard.sh (3)

1-10: Consider adding a shell directive for static analysis compliance.

Add # shellcheck shell=bash at the top to clarify the intended shell and silence SC2148 warnings.

🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


95-95: Typo in editors metadata description.

The description string contains typos: "vscodum notpadqq" should be "codium notepadqq".

-	["_checklist_editors,desc"]="Editor installation and management (vscodum notpadqq and more)."
+	["_checklist_editors,desc"]="Editor installation and management (codium notepadqq and more)."

12-37: Modular checklist logic is clear and effective.

Each checklist function dynamically builds package lists, checks install state, and delegates to the reusable helper. The approach is robust and extensible.

Minor suggestions:

  • Consider handling cases where apt-cache search returns no description (e.g., fallback to package name).
  • For large package lists, consider limiting or paginating results for better UX.

Also applies to: 51-88, 101-139

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd6c94f and b96b968.

📒 Files selected for processing (2)
  • tools/modules/functions/interface_checklist.sh (1 hunks)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/functions/interface_checklist.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


[warning] 77-77: checklist_options appears unused. Verify use (or export if used externally).

(SC2034)

tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
tools/modules/software/module_aptwizard.sh (1)

153-187: 🛠️ Refactor suggestion

Dispatcher logic is clear, but fix typo in function call and improve error handling.

  • The dispatcher parses commands and routes to the appropriate checklist function.
  • There is a typo: _checklist_eitors should be _checklist_editors (as flagged in past review comments).
  • The error message for invalid commands could be more user-friendly (add a space after the period).
  • Consider adding a default help message if no argument is provided.

Apply these fixes:

-			_checklist_eitors
+			_checklist_editors

-		echo "Invalid command.try: '${module_options["module_aptwizard,example"]}'"
+		echo "Invalid command. Try: '${module_options["module_aptwizard,example"]}'"
🧹 Nitpick comments (4)
tools/modules/software/module_aptwizard.sh (4)

51-88: Browser checklist logic is solid, but improve package existence checks and UX.

  • The function correctly builds a checklist for a fixed set of browsers, fetching descriptions and install states.
  • However, apt-cache search "^${base_package}$" may not always return a result if the package is not available in the current apt sources, leading to missing entries in the checklist.
  • Consider warning the user if a package is not found, or at least logging which packages were skipped.
  • The install state check (dpkg -l | grep -q "^ii.*$package_name") could be more robust using dpkg-query.

Example improvement for missing packages:

-		if [[ -n "$main_package" ]]; then
+		if [[ -n "$main_package" ]]; then
 			...
+		else
+			echo "Warning: Package '$base_package' not found in apt-cache."
 		fi

90-99: Metadata block for _checklist_editors is clear, but description has minor typos.

  • The description string has typos: "vscodum notpadqq" should be "codium notepadqq".
  • For clarity and professionalism, correct these typos.
-	["_checklist_editors,desc"]="Editor installation and management (vscodum notpadqq and more)."
+	["_checklist_editors,desc"]="Editor installation and management (codium, notepadqq, and more)."

101-138: Editor checklist logic is solid, but improve package existence checks and UX.

  • The logic mirrors the browser checklist, which is good for consistency.
  • The same suggestions apply: warn or log if a package is not found in apt-cache search, and use dpkg-query for install checks.
  • The comment at line 104 refers to "browser packages" instead of "editor packages"—update for clarity.
-	# List of base browser packages to manage
+	# List of base editor packages to manage

1-188: Add a shebang or shell directive for static analysis compatibility.

Shellcheck (SC2148) warns that the target shell is unknown. If this file is intended to be sourced, consider adding a comment at the top:

# shellcheck shell=bash

If it is meant to be executed directly, add a shebang (#!/bin/bash). This helps static analysis tools and improves clarity for future maintainers.

🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b96b968 and 6e1c60c.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (3)
tools/modules/software/module_aptwizard.sh (3)

1-10: Metadata block for _checklist_proftpd is clear and well-structured.

The metadata is consistent and provides useful information for documentation and grouping.

🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)


40-49: Metadata block for _checklist_browsers is clear and well-structured.

No issues found; the metadata is consistent and descriptive.


140-151: Metadata block for module_aptwizard is clear and well-structured.

No issues found; the metadata is consistent and descriptive.

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
tools/modules/software/module_aptwizard.sh (1)

177-177: ⚠️ Potential issue

Fix typo in function call: _checklist_eitors should be _checklist_editors.

There is a typo in the function call. Update it for consistency and to avoid runtime errors.

-			_checklist_eitors
+			_checklist_editors
🧹 Nitpick comments (2)
tools/modules/software/module_aptwizard.sh (2)

55-92: Efficient dynamic checklist for browsers, but check package description handling.

The checklist for browsers is well-structured, but if apt-cache search "^${base_package}$" returns multiple lines or no result, main_package may be empty or ambiguous. Consider handling cases where the package is not found or multiple matches are returned, and provide a fallback or warning to the user.


187-187: Improve error message for invalid command.

The error message for an invalid command is missing a space after the period and could be more user-friendly. Suggestion:

-		echo "Invalid command.try: '${module_options["module_aptwizard,example"]}'"
+		echo "Invalid command. Try one of: ${module_options["module_aptwizard,example"]}"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e1c60c and cdd0368.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (2)
tools/modules/software/module_aptwizard.sh (2)

23-27: Handle empty package list gracefully.

Good job checking for an empty package list and providing a user-friendly message if no ProFTPD-related packages are found. This improves UX and prevents confusion.


39-39: Verify external dependency for process_package_selection.

The function process_package_selection is called here but is not defined in this file. Ensure that the required helper script (tools/modules/functions/interface_checklist.sh) is always sourced before this function is used, or add a check to source it if not already loaded.

Would you like a code snippet to ensure the helper is sourced automatically?

@Tearran Tearran changed the title [UX/UI] Add reusable checklist helper [UX/UI] Add reusable checklist helper @coderabbitai Apr 15, 2025
@coderabbitai coderabbitai bot changed the title [UX/UI] Add reusable checklist helper @coderabbitai [UX/UI] Add reusable checklist helper Add interactive checklist shell script modules for package management Apr 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
tools/modules/software/module_aptwizard.sh (2)

177-177: ⚠️ Potential issue

Fix typo in function name: _checklist_eitors should be _checklist_editors.

The dispatcher calls _checklist_eitors, but the correct function is _checklist_editors. Update the function call and all references for consistency.

-			_checklist_eitors
+			_checklist_editors

99-99: ⚠️ Potential issue

Fix typos in editor metadata.

There are typos in the description: "codum" should be "codium" and "notpadqq" should be "notepadqq".

-	["_checklist_editors,desc"]="Editor installation and management (codum notpadqq and more)."
+	["_checklist_editors,desc"]="Editor installation and management (codium notepadqq and more)."
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cdd0368 and f081e79.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (1)
tools/modules/software/module_aptwizard.sh (1)

105-142: Approve modular checklist logic and integration with helpers.

The checklist functions for editors, browsers, and ProFTPD are well-structured, modular, and leverage reusable helpers for UI and package management. This design improves maintainability and user experience.

@Tearran Tearran changed the title [UX/UI] Add reusable checklist helper Add interactive checklist shell script modules for package management [UX/UI] Add interactive checklist shell script modules for package management Apr 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
tools/modules/software/module_aptwizard.sh (1)

1-1: ⚠️ Potential issue

Add a shebang at the top of the script.

This script is missing a shebang (#!/bin/bash or similar) at the top, which is required for shell scripts to specify the interpreter. This omission can cause issues when executing the script directly.

+#!/bin/bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🧹 Nitpick comments (3)
tools/modules/software/module_aptwizard.sh (3)

105-105: Fix typos in editor metadata.

There are typos in the editors description: "codum" should be "codium" and "notpadqq" should be "notepadqq".

-	["_checklist_editors,desc"]="Editor installation and management (codum notpadqq and more)."
+	["_checklist_editors,desc"]="Editor installation and management (codium notepadqq and more)."

91-94: Consistent empty package list handling and user feedback.

Good job handling empty package lists for browsers and editors. For even better UX, consider making the error messages consistent and more informative, e.g.,

-		echo "No $title packages found."
+		echo "No $title packages found. Please check your package sources or network connection."

Also applies to: 146-149


197-197: Improve invalid command error message for clarity.

The error message for invalid commands is missing a space and could be more user-friendly.

-		echo "Invalid command.try: '${module_options["module_aptwizard,example"]}'"
+		echo "Invalid command. Try one of: ${module_options["module_aptwizard,example"]}"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0d5c33 and a6fffff.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
tools/modules/software/module_aptwizard.sh (1)

1-1: ⚠️ Potential issue

Add a shebang at the top of the script.

This script is missing a shebang (#!/bin/bash or similar) at the top, which is required for shell scripts to specify the interpreter. This omission can cause issues when executing the script directly.

+#!/bin/bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a6fffff and c7ef88f.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (3)
tools/modules/software/module_aptwizard.sh (3)

24-27: Graceful handling of empty package lists is implemented.

The code checks for empty package lists and provides user-friendly messages, which improves UX and prevents confusion.

Also applies to: 91-94, 146-149


100-109: Metadata and naming are consistent and typo-free.

The metadata for all checklist functions and the main module is clear, accurate, and free of previous typos. Naming conventions are consistent.

Also applies to: 155-165


167-201: Modular dispatcher logic is clear and extensible.

The main dispatcher function (module_aptwizard) is well-structured, dynamically derives commands from metadata, and cleanly routes to the appropriate checklist functions. This design is maintainable and extensible.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
tools/modules/software/module_aptwizard.sh (3)

84-88: 🛠️ Refactor suggestion

Use dpkg-query for accurate package installation checks.

Repeat the same improvement for browser package checks:

-			if dpkg -l | grep -q "^ii.*$package_name"; then
+			if dpkg-query -W -f='${Status}' "$package_name" 2>/dev/null | grep -q "install ok installed"; then

139-143: 🛠️ Refactor suggestion

Use dpkg-query for accurate package installation checks.

Repeat the same improvement for editor package checks:

-			if dpkg -l | grep -q "^ii.*$package_name"; then
+			if dpkg-query -W -f='${Status}' "$package_name" 2>/dev/null | grep -q "install ok installed"; then

1-1: ⚠️ Potential issue

Add a shebang at the top of the script.

This script is missing a shebang (#!/bin/bash or similar) at the top, which is required for shell scripts to specify the interpreter. This omission can cause issues when executing the script directly.

+#!/bin/bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7ef88f and 4a43399.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (3)
tools/modules/software/module_aptwizard.sh (3)

24-27: Good handling of empty package lists.

Each checklist function checks for empty package lists and exits gracefully with a user-friendly message. This improves UX and prevents confusing empty checklists.

Also applies to: 91-94, 146-149


105-105: Metadata typos corrected.

The metadata for editors now correctly lists "codium" and "notepadqq". No further action needed.


167-201: Command dispatcher logic is clear and robust.

The module_aptwizard function cleanly dispatches commands and provides helpful usage information. This structure is maintainable and user-friendly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
tools/modules/software/module_aptwizard.sh (1)

1-1: ⚠️ Potential issue

Add a shebang at the top of the script.

This script is missing a shebang (#!/bin/bash or similar) at the top, which is required for shell scripts to specify the interpreter. This omission can cause issues when executing the script directly.

+#!/bin/bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🧹 Nitpick comments (1)
tools/modules/software/module_aptwizard.sh (1)

195-196: Improve invalid command error message for clarity.

The invalid command message could be clearer by including a space after the period and rephrasing for readability.

-		echo "Invalid command.try: '${module_options["module_aptwizard,example"]}'"
+		echo "Invalid command. Try one of: ${module_options["module_aptwizard,example"]}"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a43399 and b94d03c.

📒 Files selected for processing (1)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (3)
tools/modules/software/module_aptwizard.sh (3)

32-36: Accurate package installation checks using dpkg-query.

The use of dpkg-query -W -f='${Status}' "$package_name" for checking installation status is robust and avoids false positives compared to dpkg -l | grep. This is a best practice for precise package state detection.

Also applies to: 83-87, 137-141


24-27: Graceful handling of empty package lists.

Each checklist function checks for empty package lists and provides a user-friendly message before exiting. This improves UX and prevents confusing empty checklists.

Also applies to: 90-93, 144-147


1-200: Modular, maintainable, and user-friendly design.

The script is well-structured, modular, and leverages reusable helper functions for interactive package management. Error handling, metadata, and command dispatching are implemented cleanly, following best practices for maintainability and UX.

🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

@Tearran
Copy link
Member Author

Tearran commented Apr 15, 2025

"./bin/armbian-config" --api see_menu module_aptwizard

{6863145A-8D0D-4E75-B6AD-5A64A301B555} {720D3FAF-911E-461B-9449-0BADDCB3DC4E}

@Tearran
Copy link
Member Author

Tearran commented Apr 15, 2025

armbian-config>Software
{22D7955B-95C3-4F7B-A854-7F5BC8DAD009}

armbian-config>Software>AptWizard
or
./bin/armbian-config" --cmd AptWizard
or
./bin/armbian-config" --api see_menu module_aptwizard

{7882E1A7-B08B-466C-AA87-89E1C85B6CFC}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
tools/modules/functions/interface_checklist.sh (1)

1-1: ⚠️ Potential issue

Add a shebang for shell compatibility.

This script is missing a shebang (#!/bin/bash or similar) at the top, which is required for shell scripts to specify the interpreter. This omission can cause issues when executing the script directly or when static analysis tools attempt to lint it.

+#!/bin/bash
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64e62d1 and 1de1214.

📒 Files selected for processing (1)
  • tools/modules/functions/interface_checklist.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/functions/interface_checklist.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
coderabbitai bot added a commit that referenced this pull request Apr 16, 2025
Docstrings generation was requested by @Tearran.

* #530 (comment)

The following files were modified:

* `tools/modules/software/module_aptwizard.sh`
Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

Note

Generated docstrings for this pull request at #531

@Tearran
Copy link
Member Author

Tearran commented Apr 16, 2025

@coderabbitai generate unit testing code for this file

Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# List files changed in this PR
git diff --name-only HEAD~1 HEAD

# First, let's look at the interface_checklist.sh file content
echo "========== interface_checklist.sh content =========="
cat tools/modules/functions/interface_checklist.sh 2>/dev/null || echo "File not found"

# Then, let's look at the module_aptwizard.sh file content
echo "========== module_aptwizard.sh content =========="
cat tools/modules/software/module_aptwizard.sh 2>/dev/null || echo "File not found"

# Let's check if there's any existing testing framework or pattern in the codebase
echo "========== Looking for existing test files =========="
find . -name "*test*" -type f | grep -v "node_modules" | grep -v ".git" | head -n 10

Length of output: 13531


🏁 Script executed:

#!/bin/bash
# Check for any existing test framework
echo "========== Checking for any test framework or test files =========="
find . -path "*test*" -type f | grep -E "\.sh$" | grep -v "module_armbian_kvmtest.sh" | head -n 5

# Look for existing test patterns
echo "========== Checking for assertions or test patterns =========="
grep -r "assert" --include="*.sh" . | head -n 5

# Check how pkg_install and pkg_remove are defined (since they're used by process_package_selection)
echo "========== Checking pkg_install and pkg_remove implementation =========="
grep -r "function pkg_install" --include="*.sh" . | head -n 2
grep -r "function pkg_remove" --include="*.sh" . | head -n 2

Length of output: 707


🏁 Script executed:

#!/bin/bash
# Let's find other utility functions that might be relevant
echo "========== Finding pkg_install and pkg_remove definitions =========="
find . -type f -name "*.sh" -exec grep -l "pkg_install\|pkg_remove" {} \; | head -n 5

# Find how DIALOG is set
echo "========== Finding where DIALOG is set =========="
grep -r "DIALOG=" --include="*.sh" . | grep -v "DIALOG==" | head -n 10

# Checking interface_menu.sh since it might have similar functionality
echo "========== Checking interface_menu.sh =========="
find . -type f -name "interface_menu.sh" -exec cat {} \; | head -n 50

Length of output: 4717


🏁 Script executed:

#!/bin/bash
# Let's look at package.sh to understand pkg_install and pkg_remove functions
echo "========== Checking package.sh =========="
cat ./tools/modules/functions/package.sh 2>/dev/null || echo "File not found"

Length of output: 2765


flowchart TB
    A[Create unit tests for interface_checklist.sh] --o B[Setup testing framework]
    A --o C[Test interface_checklist function]
    A --o D[Test process_package_selection function]
    
    B --o B1[Create test environment]
    B --o B2[Create mock utilities]
    
    C --o C1[Test whiptail interface]
    C --o C2[Test dialog interface]
    C --o C3[Test read interface]
    C --o C4[Test error handling]
    
    D --o D1[Test package installation]
    D --o D2[Test package removal]
    D --o D3[Test cancellation handling]
    D --o D4[Test empty selections]
Loading

@Tearran I've created a plan to generate comprehensive unit tests for the interface_checklist.sh file. The tests will focus on:

  1. Testing interface_checklist function:

    • Testing with whiptail, dialog, and read interfaces
    • Testing user cancellation and error handling
  2. Testing process_package_selection function:

    • Testing installation of new packages
    • Testing removal of packages
    • Testing cancellation handling

The test implementation will create mock versions of dependencies like whiptail, dialog, and the package management functions (pkg_install and pkg_remove). This allows us to verify that your functions are calling the right utilities with the right parameters.

The tests will be executed in a dedicated testing environment that logs all calls to external utilities and validates expected outputs.

Would you like me to proceed with generating the detailed test code? If so, I can work with you to adapt the tests to your specific testing conventions or requirements.

(づ ◕‿◕ )づ


⏳ Preparing changes...

Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

A summary of the edits CodeRabbit can apply:

We will create a new unit test file at tools/modules/tests/test_interface_checklist.sh that sets up the testing framework with mocks and helper functions, and includes comprehensive tests for both the interface_checklist and process_package_selection functions from tools/modules/functions/interface_checklist.sh.

The unit test file for interface_checklist.sh will be created at tools/tests/test_interface_checklist.sh. This file will include the setup and teardown framework, mocks for external tools, and comprehensive tests for both the interface_checklist and process_package_selection functions.

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

➕ tools/modules/tests/test_interface_checklist.sh
#!/bin/bash
# Unit tests for interface_checklist.sh functions

# Define test directory
TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SOURCE_DIR="$TEST_DIR/../functions"

# Import functions to test
source "$SOURCE_DIR/interface_checklist.sh"

# Mock variables and functions
declare -A module_options
DIALOG=""
OUTPUTS_DIR="$TEST_DIR/outputs"
RUN_COUNT=0
MOCK_OUTPUT=""
MOCK_EXIT_CODE=0

# Create outputs directory if it doesn't exist
mkdir -p "$OUTPUTS_DIR"

# Mock dialog tools
function whiptail() {
    echo "Called whiptail with args: $*" >> "$OUTPUTS_DIR/whiptail_calls_$RUN_COUNT.log"
    echo "$MOCK_OUTPUT" >&3
    return $MOCK_EXIT_CODE
}

function dialog() {
    echo "Called dialog with args: $*" >> "$OUTPUTS_DIR/dialog_calls_$RUN_COUNT.log"
    echo "$MOCK_OUTPUT" >&2
    return $MOCK_EXIT_CODE
}

# Mock package functions
function pkg_install() {
    echo "Called pkg_install with: $*" >> "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    return 0
}

function pkg_remove() {
    echo "Called pkg_remove with: $*" >> "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"
    return 0
}

# Test helper functions
function setup_test() {
    RUN_COUNT=$((RUN_COUNT + 1))
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=0
    > "$OUTPUTS_DIR/whiptail_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/dialog_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
}

function assert_equals() {
    local expected="$1"
    local actual="$2"
    local message="${3:-Expected '$expected' but got '$actual'}"
    
    if [[ "$expected" != "$actual" ]]; then
        echo "FAIL: $message"
        return 1
    else
        echo "PASS: Values match: $expected"
        return 0
    fi
}

function assert_contains() {
    local pattern="$1"
    local file="$2"
    local message="${3:-Expected pattern '$pattern' in file $file}"
    
    if grep -q "$pattern" "$file"; then
        echo "PASS: File contains '$pattern'"
        return 0
    else
        echo "FAIL: $message"
        return 1
    fi
}

function run_all_tests() {
    local failures=0
    # Run each test function
    for test_func in $(declare -F | grep "^declare -f test_" | cut -d' ' -f3); do
        echo "Running $test_func"
        setup_test
        if ! $test_func; then
            failures=$((failures + 1))
        fi
        echo "------------------"
    done
    echo "Test Summary: $failures failures"
    return $failures
}

# Cleanup function to run at exit
function cleanup() {
    echo "Cleaning up test artifacts..."
    # Uncomment to clean output files when tests pass
    # rm -rf "$OUTPUTS_DIR"
}

# Register cleanup
trap cleanup EXIT

# ---- Unit Tests for interface_checklist function ----

function test_interface_checklist_whiptail() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"option1" "option3"'
    MOCK_EXIT_CODE=0
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify call to whiptail
    assert_contains "whiptail --title \"Test Title\" --checklist" "$OUTPUTS_DIR/whiptail_calls_$RUN_COUNT.log"
    # Verify output
    assert_equals '"option1" "option3"' "$result"
}

function test_interface_checklist_dialog() {
    DIALOG="dialog"
    MOCK_OUTPUT="option1 option3"
    MOCK_EXIT_CODE=0
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify call to dialog
    assert_contains "dialog --title \"Test Title\" --checklist" "$OUTPUTS_DIR/dialog_calls_$RUN_COUNT.log"
    # Verify output
    assert_equals "option1 option3" "$result"
}

function test_interface_checklist_read() {
    DIALOG="read"
    # Mock read command
    function read() {
        echo "1 3" >&$BASH_REMATCH
    }
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify output - for read interface, it should output option1 option3
    assert_equals " option1 option3" "$result"
}

function test_interface_checklist_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF")
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify output shows cancellation
    assert_equals "Checklist canceled." "$result"
    # Verify function returns non-zero exit code
    [[ $? -ne 0 ]] && echo "PASS: Function returned non-zero exit code" || echo "FAIL: Function did not return non-zero exit code"
}

# ---- Unit Tests for process_package_selection function ----

function test_process_package_selection_install() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"package1" "package3"'
    MOCK_EXIT_CODE=0
    # Create test options
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "OFF")
    # Call function and capture output
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
    # Verify pkg_install was called for selected packages that were OFF
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    assert_contains "Called pkg_install with: package3" "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    # Verify pkg_remove was not called for package2 which stays ON
    ! grep -q "package2" "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log" && \
        echo "PASS: pkg_remove not called for package2" || \
        echo "FAIL: pkg_remove was called for package2"
}

function test_process_package_selection_remove() {
    DIALOG="dialog"
    MOCK_OUTPUT="package1"
    MOCK_EXIT_CODE=0
    # Create test options
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "ON")
    # Call function and capture output
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
    # Verify pkg_install was called for package1
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    # Verify pkg_remove was called for package3 (ON -> OFF)
    assert_contains "Called pkg_remove with: package3" "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"
}

function test_process_package_selection_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    # Create test options
    local checklist_options=("package1" "Description 1" "ON" "package2" "Description 2" "OFF")
    # Call function and capture output
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
    # Verify no packages were installed/removed
    [[ ! -s "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log" ]] && \
        echo "PASS: pkg_install not called" || \
        echo "FAIL: pkg_install was called despite cancellation"
    [[ ! -s "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log" ]] && \
        echo "PASS: pkg_remove not called" || \
        echo "FAIL: pkg_remove was called despite cancellation"
    # Verify cancel message
    assert_contains "No changes made" "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
}

# Run all tests
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
    run_all_tests
fi
📄 tools/tests/test_interface_checklist.sh

Begin the file with a bash shebang and define the test directory and source directory. Insert the following code at the top of tools/tests/test_interface_checklist.sh:

#!/bin/bash
# Unit tests for interface_checklist.sh functions

# Define test directory
TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SOURCE_DIR="$TEST_DIR/../functions"

# Import functions to test
source "$SOURCE_DIR/interface_checklist.sh"

Add mock variables and functions to simulate external dependencies. Immediately after the imports, add:

# Mock variables and functions
declare -A module_options
DIALOG=""
OUTPUTS_DIR="$TEST_DIR/outputs"
RUN_COUNT=0
MOCK_OUTPUT=""
MOCK_EXIT_CODE=0

# Create outputs directory if it doesn't exist
mkdir -p "$OUTPUTS_DIR"

# Mock dialog tools
function whiptail() {
    echo "Called whiptail with args: $*" >> "$OUTPUTS_DIR/whiptail_calls_$RUN_COUNT.log"
    echo "$MOCK_OUTPUT" >&3
    return $MOCK_EXIT_CODE
}

function dialog() {
    echo "Called dialog with args: $*" >> "$OUTPUTS_DIR/dialog_calls_$RUN_COUNT.log"
    echo "$MOCK_OUTPUT" >&2
    return $MOCK_EXIT_CODE
}

# Mock package functions
function pkg_install() {
    echo "Called pkg_install with: $*" >> "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    return 0
}

function pkg_remove() {
    echo "Called pkg_remove with: $*" >> "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"
    return 0
}

Add helper functions for setting up tests, assertions, and running all tests. Insert the following code:

# Test helper functions
function setup_test() {
    RUN_COUNT=$((RUN_COUNT + 1))
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=0
    > "$OUTPUTS_DIR/whiptail_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/dialog_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"
    > "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
}

function assert_equals() {
    local expected="$1"
    local actual="$2"
    local message="${3:-Expected '$expected' but got '$actual'}"
    
    if [[ "$expected" != "$actual" ]]; then
        echo "FAIL: $message"
        return 1
    else
        echo "PASS: Values match: $expected"
        return 0
    fi
}

function assert_contains() {
    local pattern="$1"
    local file="$2"
    local message="${3:-Expected pattern '$pattern' in file $file}"
    
    if grep -q "$pattern" "$file"; then
        echo "PASS: File contains '$pattern'"
        return 0
    else
        echo "FAIL: $message"
        return 1
    fi
}

function run_all_tests() {
    local failures=0
    
    # Run each test function
    for test_func in $(declare -F | grep "^declare -f test_" | cut -d' ' -f3); do
        echo "Running $test_func"
        setup_test
        if ! $test_func; then
            failures=$((failures + 1))
        fi
        echo "------------------"
    done
    
    echo "Test Summary: $failures failures"
    return $failures
}

Add a cleanup function and register it using a trap. Append:

# Cleanup function to run at exit
function cleanup() {
    echo "Cleaning up test artifacts..."
    # Uncomment the line below to remove output files after tests pass
    # rm -rf "$OUTPUTS_DIR"
}

# Register cleanup
trap cleanup EXIT

Add tests for the interface_checklist function. Append the following test cases:

# Tests for interface_checklist function
function test_interface_checklist_whiptail() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"option1" "option3"'
    MOCK_EXIT_CODE=0
    
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify call to whiptail
    assert_contains "whiptail --title \"Test Title\" --checklist" "$OUTPUTS_DIR/whiptail_calls_$RUN_COUNT.log"
    
    # Verify output
    assert_equals '"option1" "option3"' "$result"
}

function test_interface_checklist_dialog() {
    DIALOG="dialog"
    MOCK_OUTPUT="option1 option3"
    MOCK_EXIT_CODE=0
    
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify call to dialog
    assert_contains "dialog --title \"Test Title\" --checklist" "$OUTPUTS_DIR/dialog_calls_$RUN_COUNT.log"
    
    # Verify output
    assert_equals "option1 option3" "$result"
}

function test_interface_checklist_read() {
    DIALOG="read"
    
    # Mock read command within this scope
    function read() {
        echo "1 3"
    }
    
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify output - for read interface, it should output " option1 option3"
    assert_equals " option1 option3" "$result"
}

function test_interface_checklist_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    
    # Create test options
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF")
    
    # Call function and capture output
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify output shows cancellation
    assert_equals "Checklist canceled." "$result"
    
    # Verify function returns non-zero exit code
    [[ $? -ne 0 ]] && echo "PASS: Function returned non-zero exit code" || echo "FAIL: Function did not return non-zero exit code"
}

Add tests for the process_package_selection function. Append:

# Tests for process_package_selection function
function test_process_package_selection_install() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"package1" "package3"'
    MOCK_EXIT_CODE=0
    
    # Create test options
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "OFF")
    
    # Call function and capture output
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
    
    # Verify pkg_install was called for selected packages that were OFF
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    assert_contains "Called pkg_install with: package3" "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    
    # Verify pkg_remove was not called for package2 which stays ON
    if ! grep -q "package2" "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"; then
        echo "PASS: pkg_remove not called for package2"
    else
        echo "FAIL: pkg_remove was called for package2"
    fi
}

function test_process_package_selection_remove() {
    DIALOG="dialog"
    MOCK_OUTPUT="package1"
    MOCK_EXIT_CODE=0
    
    # Create test options
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "ON")
    
    # Call function and capture output
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
    
    # Verify pkg_install was called for package1
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log"
    
    # Verify pkg_remove was called for package3
    assert_contains "Called pkg_remove with: package3" "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log"
}

function test_process_package_selection_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    
    # Create test options
    local checklist_options=("package1" "Description 1" "ON" "package2" "Description 2" "OFF")
    
    # Call function and capture output
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
    
    # Verify no packages were installed/removed
    if [[ ! -s "$OUTPUTS_DIR/pkg_install_calls_$RUN_COUNT.log" ]]; then
        echo "PASS: pkg_install not called"
    else
        echo "FAIL: pkg_install was called despite cancellation"
    fi
    if [[ ! -s "$OUTPUTS_DIR/pkg_remove_calls_$RUN_COUNT.log" ]]; then
        echo "PASS: pkg_remove not called"
    else
        echo "FAIL: pkg_remove was called despite cancellation"
    fi
    
    # Verify cancellation message
    assert_contains "No changes made" "$OUTPUTS_DIR/test_output_$RUN_COUNT.log"
}

At the end of the file, add a conditional block to run all tests if the script is executed directly. Append:

# Run all tests if file is executed directly
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
    run_all_tests
fi

A summary of the context that CodeRabbit is considering across the codebase:

This new unit test file at <code>tools/modules/tests/test_interface_checklist.sh</code> sets up a full testing framework for the functions defined in <code>tools/modules/functions/interface_checklist.sh</code>. It includes helper functions for test setup, assertions, cleanup, and mocks for external dialog tools (whiptail, dialog) and package functions (pkg_install, pkg_remove). Comprehensive tests are provided for both the <code>interface_checklist</code> and <code>process_package_selection</code> functions which are expected to handle normal operation and cancellation scenarios.

This unit test file is designed to validate the behavior of functions in the interface_checklist.sh module by mocking external dependencies such as dialog tools and package functions. Each test simulates different user interactions—using whiptail, dialog, read interface, and scenarios where the user cancels the operation—ensuring that expected outputs and function calls (or lack thereof) are verified. The helper functions provide a consistent setup and cleanup environment for each test case.

  • Redone edit planning - (🔄 Check again to redo again)

Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

A summary of the edits CodeRabbit can apply:

We are going to create a new unit test file at tools/modules/tests/test_interface_checklist.sh that comprehensively tests the functions in tools/modules/functions/interface_checklist.sh. This file will set up a testing framework with mocks for external dependencies (whiptail, dialog, pkg_install, pkg_remove), define helper functions for assertions and test setups, and include unit tests for both the interface_checklist and process_package_selection functions.

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

📄 tools/modules/tests/test_interface_checklist.sh
#!/bin/bash
# Unit tests for interface_checklist.sh functions

# Define test directory and set source directory
TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SOURCE_DIR="$TEST_DIR/../functions"
# Import functions from interface_checklist.sh for testing
source "$SOURCE_DIR/interface_checklist.sh"

# Define mocks and global variables
declare -A module_options
DIALOG=""
OUTPUTS_DIR="$TEST_DIR/outputs"
RUN_COUNT=0
MOCK_OUTPUT=""
MOCK_EXIT_CODE=0

# Create an outputs directory for logging mock calls
mkdir -p "$OUTPUTS_DIR"

# Mock implementations for dialog tools
whiptail() {
    echo "Called whiptail with args: $*" >> "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    echo "$MOCK_OUTPUT" >&3
    return $MOCK_EXIT_CODE
}

dialog() {
    echo "Called dialog with args: $*" >> "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    echo "$MOCK_OUTPUT" >&2
    return $MOCK_EXIT_CODE
}

# Mock implementations for package management functions
pkg_install() {
    echo "Called pkg_install with: $*" >> "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    return 0
}

pkg_remove() {
    echo "Called pkg_remove with: $*" >> "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
    return 0
}

# Define helper functions for test setup, assertions, and cleanup

setup_test() {
    RUN_COUNT=$((RUN_COUNT + 1))
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=0
    > "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
}

assert_equals() {
    local expected="$1"
    local actual="$2"
    local message="${3:-Expected '$expected' but got '$actual'}"
    if [[ "$expected" != "$actual" ]]; then
        echo "FAIL: $message"
        return 1
    else
        echo "PASS: Values match: $expected"
        return 0
    fi
}

assert_contains() {
    local pattern="$1"
    local file="$2"
    local message="${3:-Expected pattern '$pattern' in file $file}"
    if grep -q "$pattern" "$file"; then
        echo "PASS: File contains '$pattern'"
        return 0
    else
        echo "FAIL: $message"
        return 1
    fi
}

run_all_tests() {
    local failures=0
    for test_func in $(declare -F | awk '{print $3}' | grep "^test_"); do
        echo "Running $test_func"
        setup_test
        if ! $test_func; then
            failures=$((failures + 1))
        fi
        echo "------------------"
    done
    echo "Test Summary: $failures failures"
    return $failures
}

cleanup() {
    echo "Cleaning up test artifacts..."
    # Optionally remove test output files: rm -rf "$OUTPUTS_DIR"
}
trap cleanup EXIT

# Unit tests for interface_checklist function

test_interface_checklist_whiptail() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"option1" "option3"'
    MOCK_EXIT_CODE=0
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify that whiptail was invoked correctly
    assert_contains "whiptail --title \"Test Title\" --checklist" "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    # Verify that the function output matches expected options
    assert_equals '"option1" "option3"' "$result"
}

test_interface_checklist_dialog() {
    DIALOG="dialog"
    MOCK_OUTPUT="option1 option3"
    MOCK_EXIT_CODE=0
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify that dialog was invoked correctly
    assert_contains "dialog --title \"Test Title\" --checklist" "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    # Verify that the function output matches expected options
    assert_equals "option1 option3" "$result"
}

test_interface_checklist_read() {
    DIALOG="read"
    # Temporarily override the read command within this test
    function read() { echo "1 3"; }
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # For the read interface, the output is expected to be " option1 option3"
    assert_equals " option1 option3" "$result"
}

test_interface_checklist_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify that cancellation is handled as expected
    assert_equals "Checklist canceled." "$result"
    [[ $? -ne 0 ]] && echo "PASS: Function returned non-zero exit code" || echo "FAIL: Function did not return non-zero exit code"
}

# Unit tests for process_package_selection function

test_process_package_selection_install() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"package1" "package3"'
    MOCK_EXIT_CODE=0
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "OFF")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    
    # Verify that pkg_install was called for packages selected for installation
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    assert_contains "Called pkg_install with: package3" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
}

test_process_package_selection_remove() {
    DIALOG="dialog"
    MOCK_OUTPUT="package1"
    MOCK_EXIT_CODE=0
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "ON")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    
    # Verify that pkg_install was called for removal scenario where package status changes
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    # Verify that pkg_remove was invoked for the unselected package
    assert_contains "Called pkg_remove with: package3" "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
}

test_process_package_selection_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    local checklist_options=("package1" "Description 1" "ON" "package2" "Description 2" "OFF")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    
    # Verify that no installation or removal occurred when cancelled
    if [[ ! -s "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log" ]]; then
        echo "PASS: pkg_install not called"
    else
        echo "FAIL: pkg_install was called despite cancellation"
    fi
    if [[ ! -s "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log" ]]; then
        echo "PASS: pkg_remove not called"
    else
        echo "FAIL: pkg_remove was called despite cancellation"
    fi
    assert_contains "No changes made" "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
}

# Execute all tests if this file is run directly
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
    run_all_tests
fi

A summary of the context that CodeRabbit is considering across the codebase:

We executed a shell search for "interface_checklist" in tools/modules/functions/interface_checklist.sh and found no output, which confirms that our unit test file correctly imports and targets the appropriate functions for testing. The search ensures our tests are referencing the correct functions for a seamless integration.
  • Created local commit - (🔄 Check again to try again)

Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

Creating a commit in the current branch to apply the edits...

The changes have been committed to the current branch. Commit ID: e8c6123fbb493083ecdf7532f46a43497e10e63d

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5306c0e and e8c6123.

📒 Files selected for processing (1)
  • tools/modules/tests/test_interface_checklist.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/tests/test_interface_checklist.sh

[warning] 11-11: module_options appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 27-27: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)


[warning] 28-28: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)


[warning] 29-29: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)


[warning] 30-30: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)


[warning] 31-31: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).

(SC2188)


[warning] 161-161: DIALOG appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 162-162: MOCK_OUTPUT appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 163-163: MOCK_EXIT_CODE appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 164-164: checklist_options appears unused. Verify use (or export if used externally).

(SC2034)

🪛 GitHub Actions: Coding style check
tools/modules/tests/test_interface_checklist.sh

[error] 24-31: Wrong indentation type (spaces instead of tabs).


[error] 62-71: Wrong indentation type (spaces instead of tabs).


[error] 124-130: Wrong indentation type (spaces instead of tabs).


[error] 153-153: Trailing whitespace.


[error] 153-157: Wrong indentation type (spaces instead of tabs).


[error] 166-178: Wrong indentation type (spaces instead of tabs).

🔇 Additional comments (1)
tools/modules/tests/test_interface_checklist.sh (1)

11-11: Unused variable warnings (minor).

ShellCheck flags module_options, DIALOG, MOCK_OUTPUT, MOCK_EXIT_CODE, and checklist_options as unused in some scopes. In a test context, these are often set for use by sourced or mocked functions, so these warnings can be safely ignored unless you want to further clean up the code.

Also applies to: 161-161, 162-162, 163-163, 164-164

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 11-11: module_options appears unused. Verify use (or export if used externally).

(SC2034)

Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

A summary of the edits CodeRabbit can apply:

We will create a new unit test file at tools/modules/tests/test_interface_checklist.sh that comprehensively tests the functions defined in tools/modules/functions/interface_checklist.sh, including the interface_checklist and process_package_selection functions. The tests will use mocks for external dependencies (dialog tools such as whiptail, dialog, and package management functions) and include helper functions to set up tests, assert expectations, run tests, and perform cleanup.

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

➕ tools/modules/tests/test_interface_checklist.sh
#!/bin/bash
# Unit tests for interface_checklist.sh functions

# Define test directory and set source directory
TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SOURCE_DIR="$TEST_DIR/../functions"
# Import functions from interface_checklist.sh for testing
source "$SOURCE_DIR/interface_checklist.sh"

# Define mocks and global variables
declare -A module_options
DIALOG=""
OUTPUTS_DIR="$TEST_DIR/outputs"
RUN_COUNT=0
MOCK_OUTPUT=""
MOCK_EXIT_CODE=0

# Create an outputs directory for logging mock calls
mkdir -p "$OUTPUTS_DIR"

# Define helper functions for test setup, assertions, and cleanup
setup_test() {
    RUN_COUNT=$((RUN_COUNT + 1))
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=0
    > "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
}

assert_equals() {
    local expected="$1"
    local actual="$2"
    local message="${3:-Expected '$expected' but got '$actual'}"
    if [[ "$expected" != "$actual" ]]; then
        echo "FAIL: $message"
        return 1
    else
        echo "PASS: Values match: $expected"
        return 0
    fi
}

assert_contains() {
    local pattern="$1"
    local file="$2"
    local message="${3:-Expected pattern '$pattern' in file $file}"
    if grep -q "$pattern" "$file"; then
        echo "PASS: File contains '$pattern'"
        return 0
    else
        echo "FAIL: $message"
        return 1
    fi
}

run_all_tests() {
    local failures=0
    for test_func in $(declare -F | awk '{print $3}' | grep "^test_"); do
        echo "Running $test_func"
        setup_test
        if ! $test_func; then
            failures=$((failures + 1))
        fi
        echo "------------------"
    done
    echo "Test Summary: $failures failures"
    return $failures
}

cleanup() {
    echo "Cleaning up test artifacts..."
    # Optionally remove test output files: rm -rf "$OUTPUTS_DIR"
}
trap cleanup EXIT

# Mock implementations for external dependencies
whiptail() {
    echo "Called whiptail with args: $*" >> "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    echo "$MOCK_OUTPUT" >&3
    return $MOCK_EXIT_CODE
}

dialog() {
    echo "Called dialog with args: $*" >> "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    echo "$MOCK_OUTPUT" >&2
    return $MOCK_EXIT_CODE
}

pkg_install() {
    echo "Called pkg_install with: $*" >> "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    return 0
}

pkg_remove() {
    echo "Called pkg_remove with: $*" >> "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
    return 0
}

# Unit tests for interface_checklist function

test_interface_checklist_whiptail() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"option1" "option3"'
    MOCK_EXIT_CODE=0
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify that whiptail was invoked correctly
    assert_contains "whiptail --title \"Test Title\" --checklist" "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    # Verify that the function output matches expected options
    assert_equals '"option1" "option3"' "$result"
}

test_interface_checklist_dialog() {
    DIALOG="dialog"
    MOCK_OUTPUT="option1 option3"
    MOCK_EXIT_CODE=0
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    
    # Verify that dialog was invoked correctly
    assert_contains "dialog --title \"Test Title\" --checklist" "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    # Verify that the function output matches expected options
    assert_equals "option1 option3" "$result"
}

test_interface_checklist_read() {
    DIALOG="read"
    # Temporarily override the read command within this test
    function read() { echo "1 3"; }
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # For the read interface, the output is expected to be " option1 option3"
    assert_equals " option1 option3" "$result"
}

test_interface_checklist_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify that cancellation is handled as expected
    assert_equals "Checklist canceled." "$result"
    [[ $? -ne 0 ]] && echo "PASS: Function returned non-zero exit code" || echo "FAIL: Function did not return non-zero exit code"
}

# Unit tests for process_package_selection function

test_process_package_selection_install() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"package1" "package3"'
    MOCK_EXIT_CODE=0
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "OFF")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    
    # Verify that pkg_install was called for packages selected for installation
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    assert_contains "Called pkg_install with: package3" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
}

test_process_package_selection_remove() {
    DIALOG="dialog"
    MOCK_OUTPUT="package1"
    MOCK_EXIT_CODE=0
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "ON")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    
    # Verify that pkg_install was called for removal scenario where package status changes
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    # Verify that pkg_remove was invoked for the unselected package
    assert_contains "Called pkg_remove with: package3" "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
}

test_process_package_selection_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    local checklist_options=("package1" "Description 1" "ON" "package2" "Description 2" "OFF")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    
    # Verify that no installation or removal occurred when cancelled
    if [[ ! -s "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log" ]]; then
        echo "PASS: pkg_install not called"
    else
        echo "FAIL: pkg_install was called despite cancellation"
    fi
    if [[ ! -s "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log" ]]; then
        echo "PASS: pkg_remove not called"
    else
        echo "FAIL: pkg_remove was called despite cancellation"
    fi
    assert_contains "No changes made" "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
}

# Execute all tests if this file is run directly
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
    run_all_tests
fi

A summary of the context that CodeRabbit is considering across the codebase:

This context confirms that the interface_checklist functions being tested are defined within the file tools/modules/functions/interface_checklist.sh. An initial grep search did not produce any conflicts with our test function names.
The file structure context shows that the tests will reside in tools/modules/tests and that the functions under test are in tools/modules/functions. This ensures our test file is correctly placed for integration into the repository.
  • Redone edit planning - (🔄 Check again to redo again)

Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

A summary of the edits CodeRabbit can apply:

We will create a new file at tools/modules/tests/test_interface_checklist.sh that implements comprehensive unit tests for the functions defined in tools/modules/functions/interface_checklist.sh. The tests include mocks for external dependencies (whiptail, dialog, pkg_install, pkg_remove), helper functions for setup and assertions, and unit tests covering normal operation and cancellation scenarios for both interface_checklist and process_package_selection.

A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets):

📄 tools/modules/tests/test_interface_checklist.sh
#!/bin/bash
# Unit tests for interface_checklist.sh functions

# Define test directory and source directory relative to this file.
TEST_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
SOURCE_DIR="$TEST_DIR/../functions"

# Import functions from interface_checklist.sh for testing.
source "$SOURCE_DIR/interface_checklist.sh"

# Define mocks and global variables.
declare -A module_options
DIALOG=""
OUTPUTS_DIR="$TEST_DIR/outputs"
RUN_COUNT=0
MOCK_OUTPUT=""
MOCK_EXIT_CODE=0

# Create an outputs directory for logging mock calls.
mkdir -p "$OUTPUTS_DIR"

# --- Helper Functions ---
# setup_test: resets global mocks and log files before each test.
setup_test() {
    RUN_COUNT=$((RUN_COUNT + 1))
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=0
    > "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
    > "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
}

# assert_equals: checks if expected output equals the actual output.
assert_equals() {
    local expected="$1"
    local actual="$2"
    local message="${3:-Expected '$expected' but got '$actual'}"
    if [[ "$expected" != "$actual" ]]; then
        echo "FAIL: $message"
        return 1
    else
        echo "PASS: Values match: $expected"
        return 0
    fi
}

# assert_contains: checks if a file contains a given pattern.
assert_contains() {
    local pattern="$1"
    local file="$2"
    local message="${3:-Expected pattern '$pattern' in file $file}"
    if grep -q "$pattern" "$file"; then
        echo "PASS: File contains '$pattern'"
        return 0
    else
        echo "FAIL: $message"
        return 1
    fi
}

# run_all_tests: finds and executes all test functions defined in this script.
run_all_tests() {
    local failures=0
    for test_func in $(declare -F | awk '{print $3}' | grep "^test_"); do
        echo "Running $test_func"
        setup_test
        if ! $test_func; then
            failures=$((failures + 1))
        fi
        echo "------------------"
    done
    echo "Test Summary: $failures failures"
    return $failures
}

# cleanup: cleans up test artifacts on exit.
cleanup() {
    echo "Cleaning up test artifacts..."
    # Uncomment the next line to remove test output files after tests pass.
    # rm -rf "$OUTPUTS_DIR"
}
trap cleanup EXIT

# --- Mock Implementations for External Dependencies ---
whiptail() {
    echo "Called whiptail with args: $*" >> "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    echo "$MOCK_OUTPUT" >&3
    return $MOCK_EXIT_CODE
}

dialog() {
    echo "Called dialog with args: $*" >> "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    echo "$MOCK_OUTPUT" >&2
    return $MOCK_EXIT_CODE
}

pkg_install() {
    echo "Called pkg_install with: $*" >> "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    return 0
}

pkg_remove() {
    echo "Called pkg_remove with: $*" >> "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
    return 0
}

# --- Unit Tests for interface_checklist Function ---
test_interface_checklist_whiptail() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"option1" "option3"'
    MOCK_EXIT_CODE=0
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify that whiptail was invoked correctly.
    assert_contains "whiptail --title \"Test Title\" --checklist" "$OUTPUTS_DIR/whiptail_calls_${RUN_COUNT}.log"
    # Verify expected output.
    assert_equals '"option1" "option3"' "$result"
}

test_interface_checklist_dialog() {
    DIALOG="dialog"
    MOCK_OUTPUT="option1 option3"
    MOCK_EXIT_CODE=0
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify that dialog was invoked correctly.
    assert_contains "dialog --title \"Test Title\" --checklist" "$OUTPUTS_DIR/dialog_calls_${RUN_COUNT}.log"
    # Verify expected output.
    assert_equals "option1 option3" "$result"
}

test_interface_checklist_read() {
    DIALOG="read"
    # Temporarily override the read command within this test.
    function read() { echo "1 3"; }
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF" "option3" "Description 3" "ON")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # For the read interface, expect output with a leading space.
    assert_equals " option1 option3" "$result"
}

test_interface_checklist_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    local checklist_options=("option1" "Description 1" "ON" "option2" "Description 2" "OFF")
    local result
    result=$(interface_checklist "Test Title" "Select options:" checklist_options)
    # Verify cancellation is handled.
    assert_equals "Checklist canceled." "$result"
    # Verify the function returns a non-zero exit code.
    if [[ $? -ne 0 ]]; then
        echo "PASS: Function returned non-zero exit code"
    else
        echo "FAIL: Function did not return non-zero exit code"
    fi
}

# --- Unit Tests for process_package_selection Function ---
test_process_package_selection_install() {
    DIALOG="whiptail"
    MOCK_OUTPUT='"package1" "package3"'
    MOCK_EXIT_CODE=0
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "OFF")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    # Verify pkg_install was called for packages needing installation.
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    assert_contains "Called pkg_install with: package3" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
}

test_process_package_selection_remove() {
    DIALOG="dialog"
    MOCK_OUTPUT="package1"
    MOCK_EXIT_CODE=0
    local checklist_options=("package1" "Description 1" "OFF" "package2" "Description 2" "ON" "package3" "Description 3" "ON")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    # Verify pkg_install was called.
    assert_contains "Called pkg_install with: package1" "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log"
    # Verify pkg_remove was called for the package to remove.
    assert_contains "Called pkg_remove with: package3" "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log"
}

test_process_package_selection_cancel() {
    DIALOG="whiptail"
    MOCK_OUTPUT=""
    MOCK_EXIT_CODE=1
    local checklist_options=("package1" "Description 1" "ON" "package2" "Description 2" "OFF")
    process_package_selection "Test" "Select packages:" checklist_options 2>&1 | tee "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
    # Check that neither pkg_install nor pkg_remove were called.
    if [[ ! -s "$OUTPUTS_DIR/pkg_install_calls_${RUN_COUNT}.log" ]]; then
        echo "PASS: pkg_install not called"
    else
        echo "FAIL: pkg_install was called despite cancellation"
    fi
    if [[ ! -s "$OUTPUTS_DIR/pkg_remove_calls_${RUN_COUNT}.log" ]]; then
        echo "PASS: pkg_remove not called"
    else
        echo "FAIL: pkg_remove was called despite cancellation"
    fi
    # Verify output contains a cancellation message.
    assert_contains "No changes made" "$OUTPUTS_DIR/test_output_${RUN_COUNT}.log"
}

# --- Execute All Tests ---
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
    run_all_tests
fi

A summary of the context that CodeRabbit is considering across the codebase:

This unit test file is designed based on our codebase structure where interface_checklist.sh is located in tools/modules/functions. Our tests mock external dependencies (whiptail, dialog, pkg_install, pkg_remove) and simulate different user interactions to thoroughly verify the functionality of interface_checklist and process_package_selection.

  • ✅ Create PR with these edits

  • 📋 Get copyable edits

  • 📌 Create commit in current branch

  • 🔄 Try a different approach

⏪ Back to ordinary chat. There will be no more planning in this conversation.

@Tearran
Copy link
Member Author

Tearran commented Apr 16, 2025

Sorry, got sidetracked with the proof of concept use case. This may have accidentally made a practical use or not , but the included humorously named "aptwizard" installer was only intended to use as a particle way to use the UX/UI components. Similarly as was the module for UX/UI compositeness for #529 .

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
tools/modules/software/module_aptwizard.sh (1)

1-1: ⚠️ Potential issue

Missing shebang at the top of the script.

This script is missing a shebang (#!/bin/bash or similar) at the top, which is required for shell scripts to specify the interpreter. This omission can cause issues when executing the script directly.

+#!/bin/bash
 module_options+=(
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🧹 Nitpick comments (5)
tools/modules/software/module_aptwizard.sh (5)

159-195: Consider reducing code duplication between editors and imaging functions.

The _checklist_imaging and _checklist_editors functions are nearly identical. Consider creating a helper function to reduce duplication.

+# Helper function for package checklist generation
+function _generate_package_checklist() {
+    local title="$1"
+    local feature="$2"
+    local self="${module_options["$feature,feature"]}"
+    local _packages
+    IFS=' ' read -r -a _packages <<< "${module_options["$self,example"]}"
+
+    # Manage package installation/removal
+    echo "Fetching $title package details..."
+
+    # Prepare checklist options dynamically with descriptions
+    local checklist_options=()
+    for base_package in "${_packages[@]}"; do
+        # Find the main package and exclude auxiliary or irrelevant ones
+        local main_package
+        main_package=$(apt-cache search "^${base_package}$" | awk -F' - ' '{print $1 " - " $2}')
+
+        # Check if the main package exists and fetch its description
+        if [[ -n "$main_package" ]]; then
+            local package_name package_description
+            package_name=$(echo "$main_package" | awk -F' - ' '{print $1}')
+            package_description=$(echo "$main_package" | awk -F' - ' '{print $2}')
+
+            # Check if the package is installed and set its state
+            if dpkg-query -W -f='${Status}' "$package_name" 2>/dev/null | grep -q "^install ok installed$"; then
+                checklist_options+=("$package_name" "$package_description" "ON")
+            else
+                checklist_options+=("$package_name" "$package_description" "OFF")
+            fi
+        fi
+    done
+    if [[ ${#checklist_options[@]} -eq 0 ]]; then
+        echo "No $title packages found."
+        return 1
+    fi
+
+    process_package_selection "$title" "Select packages to install/remove:" checklist_options[@]
+}
+
 function _checklist_editors() {
-    local title="Editors"
-    local self="${module_options["_checklist_editors,feature"]}"
-    local _packages
-    IFS=' ' read -r -a _packages <<< "${module_options["$self,example"]}"
-
-    # Manage editor installation/removal
-    echo "Fetching $title package details..."
-
-    # Prepare checklist options dynamically with descriptions
-    local checklist_options=()
-    for base_package in "${_packages[@]}"; do
-        # Find the main package and exclude auxiliary or irrelevant ones
-        local main_package
-        main_package=$(apt-cache search "^${base_package}$" | awk -F' - ' '{print $1 " - " $2}')
-
-        # Check if the main package exists and fetch its description
-        if [[ -n "$main_package" ]]; then
-            local package_name package_description
-            package_name=$(echo "$main_package" | awk -F' - ' '{print $1}')
-            package_description=$(echo "$main_package" | awk -F' - ' '{print $2}')
-
-            # Check if the package is installed and set its state
-            if dpkg-query -W -f='${Status}' "$package_name" 2>/dev/null | grep -q "^install ok installed$"; then
-                checklist_options+=("$package_name" "$package_description" "ON")
-            else
-                checklist_options+=("$package_name" "$package_description" "OFF")
-            fi
-        fi
-    done
-    if [[ ${#checklist_options[@]} -eq 0 ]]; then
-        echo "No $title packages found."
-        return 1
-    fi
-
-    process_package_selection "$title" "Select packages to install/remove:" checklist_options[@]
+    _generate_package_checklist "Editors" "_checklist_editors"
 }

 function _checklist_imaging() {
-    local title="Imaging"
-    local self="${module_options["_checklist_imaging,feature"]}"
-    local _packages
-    IFS=' ' read -r -a _packages <<< "${module_options["$self,example"]}"
-
-    # Manage editor installation/removal
-    echo "Fetching $title package details..."
-
-    # Prepare checklist options dynamically with descriptions
-    local checklist_options=()
-    for base_package in "${_packages[@]}"; do
-        # Find the main package and exclude auxiliary or irrelevant ones
-        local main_package
-        main_package=$(apt-cache search "^${base_package}$" | awk -F' - ' '{print $1 " - " $2}')
-
-        # Check if the main package exists and fetch its description
-        if [[ -n "$main_package" ]]; then
-            local package_name package_description
-            package_name=$(echo "$main_package" | awk -F' - ' '{print $1}')
-            package_description=$(echo "$main_package" | awk -F' - ' '{print $2}')
-
-            # Check if the package is installed and set its state
-            if dpkg-query -W -f='${Status}' "$package_name" 2>/dev/null | grep -q "^install ok installed$"; then
-                checklist_options+=("$package_name" "$package_description" "ON")
-            else
-                checklist_options+=("$package_name" "$package_description" "OFF")
-            fi
-        fi
-    done
-    if [[ ${#checklist_options[@]} -eq 0 ]]; then
-        echo "No $title packages found."
-        return 1
-    fi
-
-    process_package_selection "$title" "Select packages to install/remove:" checklist_options[@]
+    _generate_package_checklist "Imaging" "_checklist_imaging"
 }

166-166: Fix comment in _checklist_imaging.

The comment incorrectly mentions "editor installation/removal" but the function is for imaging tools.

-    # Manage editor installation/removal
+    # Manage imaging tools installation/removal

106-106: Group should be 'editors' not 'Internet' for _checklist_editors.

The group for editors should be consistent with the function's purpose.

-    ["_checklist_editors,group"]="Internet"
+    ["_checklist_editors,group"]="editors"

155-155: Group should be 'imaging' not 'Internet' for _checklist_imaging.

The group for imaging tools should be consistent with the function's purpose.

-    ["_checklist_imaging,group"]="Internet"
+    ["_checklist_imaging,group"]="imaging"

209-209: Fix typo in comment.

There's a typo in the comment before the module_aptwizard function.

-# Scafold for software module tites
+# Scaffold for software module titles
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e8c6123 and 4c06e60.

📒 Files selected for processing (2)
  • tests/aptwizard.conf (1 hunks)
  • tools/modules/software/module_aptwizard.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
tools/modules/software/module_aptwizard.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (5)
tests/aptwizard.conf (1)

1-8: Basic test configuration looks good.

The test configuration is properly structured with appropriate variables (ENABLED, RELEASE, TESTNAME) and a testcase function that verifies the module's help command works correctly.

tools/modules/software/module_aptwizard.sh (4)

12-40: LGTM: ProFTPD checklist function implementation is solid.

Good implementation with proper error handling for empty package lists and accurate package status checking using dpkg-query.


54-96: LGTM: Browsers checklist function looks good.

The function properly handles a predefined list of browser packages with appropriate error checking and package status verification.


109-146: LGTM: Editors checklist function effectively leverages metadata.

The implementation smartly uses the example field in metadata to define the package list, making it more maintainable.


210-248: LGTM: Command dispatcher is well implemented.

The main module_aptwizard function effectively routes commands to the appropriate checklist functions and provides a useful help output.

@Tearran Tearran marked this pull request as draft April 17, 2025 15:30
@Tearran Tearran added Discussion Being discussed - Voice your opinions :) Framework Framework components and removed Ready to merge Reviewed, tested and ready for merge labels Apr 17, 2025
@Tearran
Copy link
Member Author

Tearran commented Apr 19, 2025

Moved web browsers to [Add] Web Browsers and list #534

@Tearran Tearran mentioned this pull request Apr 26, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Being discussed - Voice your opinions :) Framework Framework components size/large PR with 250 lines or more Unit Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants