chore: Add unit tests #461
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
added
the
stale
MaxymVlasov
removed
the
stale
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
added
the
stale
MaxymVlasov
removed
the
stale
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
added
the
stale
MaxymVlasov
added
wip
MaxymVlasov
added
on-hold
WalkthroughA new shell test script was added for shunit2 to test functions in the Changes
Assessment against linked issues
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/shunit2/_common_test.sh (4)
13-14: Fix typo in comment
The word “clolrs” should be “colors” for clarity.- # Disable hooks clolrs + # Disable hook colors
21-24: Remove commented-out dead code
Thetest__initializestub is empty and commented out. Consider deleting it to keep the test file clean.
25-76: Split the large parsing test into focused subtests
This function covers multiple assertions on different arrays (ARGS,HOOK_CONFIG,TF_INIT_ARGS,ENV_VARS,FILES). Extracting each section into its own test function will improve readability, ease debugging on failure, and allow isolated maintenance.
97-98: Check for shunit2 availability
Sourcingshunit2without verifying its presence can lead to a confusing “file not found” error. Consider adding a guard with a clear message ifshunit2isn’t onPATH.-. shunit2 +command -v shunit2 >/dev/null 2>&1 || { + echo "ERROR: shunit2 not found in PATH; please install and retry." >&2 + exit 1 +} +. shunit2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/shunit2/_common_test.sh(1 hunks)
🔇 Additional comments (2)
tests/shunit2/_common_test.sh (2)
78-95: Approve parsing-and-export test
Thetest__parse_and_export_env_varsfunction correctly sets upARGS, invokescommon::parse_and_export_env_vars, and verifies the placeholders are expanded. This covers the intended functionality well.
17-20:❓ Verification inconclusive
Verify HOOK_ID evaluation
The test asserts$HOOK_IDequals_common_test, which depends on howcommon.shsetsHOOK_IDwhen sourced. Please confirm that sourcing via the test harness produces the expected value and that this behavior won’t change unintentionally.
🏁 Script executed:
#!/bin/bash # Verify where and how HOOK_ID is set in common.sh rg -n "HOOK_ID" hooks/_common.shLength of output: 87
Could you please show how
hooks/_common.shis being sourced intests/shunit2/_common_test.sh(and how the test is executed under Shunit2)? Specifically, I’d like to confirm whether$0insidehooks/_common.shreally points to_common_test.sh(not the Shunit2 runner), so thatHOOK_ID=${0##*/}yields_common_test.sh→_common_test. Please share the snippet around thesource(or.) invocation and any invocation line for Shunit2 that runs this test.
| function oneTimeSetUp { | ||
| # Load stuff for tests. | ||
| . ../../hooks/_common.sh | ||
| # Global ENV vars in --args |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make hook script sourcing path robust
Using a relative path (../../hooks/_common.sh) assumes the test is always run from tests/shunit2. To avoid breakage if the working directory changes, compute the script’s directory via ${BASH_SOURCE[0]} and source from there.
function oneTimeSetUp {
- . ../../hooks/_common.sh
+ local script_dir
+ script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ . "$script_dir"/../../hooks/_common.sh
# Global ENV vars in --args📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function oneTimeSetUp { | |
| # Load stuff for tests. | |
| . ../../hooks/_common.sh | |
| # Global ENV vars in --args | |
| function oneTimeSetUp { | |
| # Load stuff for tests. | |
| local script_dir | |
| script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| . "$script_dir"/../../hooks/_common.sh | |
| # Global ENV vars in --args | |
| } |
🤖 Prompt for AI Agents
In tests/shunit2/_common_test.sh around lines 6 to 9, the script sources
../../hooks/_common.sh using a relative path, which breaks if the working
directory changes. Modify the script to determine its own directory using
${BASH_SOURCE[0]} and then source _common.sh relative to that directory to
ensure the path is always correct regardless of where the test is run from.
Put an
xinto the box if that apply:Description of your changes
Closes #303
How can we test changes
shunit2from https://github.com/kward/shunit2shunit2to PATHpre-commit-terraform/tests/shunit2./_common_test.shSummary by CodeRabbit