-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1560] Removed obsolete BATS test helpers. #2076
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
Conversation
WalkthroughThe PR refactors BATS test infrastructure by renaming helper functions to use a fixture-oriented naming convention (prepare_* → fixture_*), introduces new fixture utilities for SSH keys, Docker config, and Robo tooling, removes three complete helper files (CircleCI, deployment, workflow), and updates dependent unit test files to use the new fixture APIs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
.vortex/tests/bats/_helper.bash(5 hunks).vortex/tests/bats/_helper.circleci.bash(0 hunks).vortex/tests/bats/_helper.deployment.bash(0 hunks).vortex/tests/bats/_helper.workflow.bash(0 hunks).vortex/tests/bats/unit/deploy-artifact.bats(2 hunks).vortex/tests/bats/unit/deploy-container-registry.bats(3 hunks).vortex/tests/bats/unit/deploy-lagoon.bats(10 hunks).vortex/tests/bats/unit/deploy-webhook.bats(0 hunks).vortex/tests/bats/unit/download-db-lagoon.bats(3 hunks).vortex/tests/bats/unit/login-container-registry.bats(1 hunks).vortex/tests/bats/unit/setup-ssh.bats(11 hunks)
💤 Files with no reviewable changes (4)
- .vortex/tests/bats/unit/deploy-webhook.bats
- .vortex/tests/bats/_helper.circleci.bash
- .vortex/tests/bats/_helper.deployment.bash
- .vortex/tests/bats/_helper.workflow.bash
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/tests/bats/unit/deploy-artifact.bats.vortex/tests/bats/unit/setup-ssh.bats.vortex/tests/bats/unit/download-db-lagoon.bats.vortex/tests/bats/unit/deploy-lagoon.bats.vortex/tests/bats/_helper.bash.vortex/tests/bats/unit/deploy-container-registry.bats.vortex/tests/bats/unit/login-container-registry.bats
📚 Learning: 2025-05-29T12:15:32.188Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
Applied to files:
.vortex/tests/bats/unit/setup-ssh.bats.vortex/tests/bats/unit/download-db-lagoon.bats.vortex/tests/bats/unit/deploy-container-registry.bats
🧬 Code graph analysis (6)
.vortex/tests/bats/unit/deploy-artifact.bats (1)
.vortex/tests/bats/_helper.bash (3)
fixture_ssh_key_prepare(249-253)fixture_ssh_key(255-258)fixture_robo(271-280)
.vortex/tests/bats/unit/setup-ssh.bats (1)
.vortex/tests/bats/_helper.bash (3)
fixture_ssh_key_prepare(249-253)fixture_ssh_key(255-258)fixture_ssh_key_with_suffix(260-263)
.vortex/tests/bats/unit/download-db-lagoon.bats (1)
.vortex/tests/bats/_helper.bash (1)
fixture_ssh_key_prepare(249-253)
.vortex/tests/bats/unit/deploy-lagoon.bats (1)
.vortex/tests/bats/_helper.bash (2)
fixture_ssh_key_prepare(249-253)fixture_ssh_key(255-258)
.vortex/tests/bats/unit/deploy-container-registry.bats (1)
.vortex/tests/bats/_helper.bash (1)
fixture_docker_config_file(265-269)
.vortex/tests/bats/unit/login-container-registry.bats (1)
.vortex/tests/bats/_helper.bash (1)
fixture_docker_config_file(265-269)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-docs
🔇 Additional comments (41)
.vortex/tests/bats/_helper.bash (12)
30-30: LGTM!Minor text update for clarity.
142-146: LGTM!Consistent fixture-based naming applied in setup phase.
166-166: LGTM!Teardown correctly updated to use the renamed restore function.
171-171: LGTM!Header appropriately renamed to reflect fixture-oriented organization.
175-192: LGTM!Clean rename from
prepare_local_repotofixture_local_repowith implementation unchanged.
194-196: LGTM!Function renamed consistently with the fixture-based convention.
198-240: LGTM!Function renamed consistently with the fixture-based convention.
242-247: LGTM!Function renamed consistently with the fixture-based convention.
249-263: LGTM!Well-structured SSH key fixture utilities that provide both default and customizable key generation.
265-269: LGTM!Docker config fixture correctly creates mock authentication configuration.
271-280: LGTM!Robo fixture properly sets up mock binaries for both robo and git-artifact.
282-296: LGTM!New utility function
git_initprovides clean git repository initialization with optional push configuration..vortex/tests/bats/unit/download-db-lagoon.bats (3)
41-41: LGTM!Fixture function renamed consistently with the new fixture-based convention.
94-94: LGTM!Fixture function renamed consistently with the new fixture-based convention.
147-147: LGTM!Fixture function renamed consistently with the new fixture-based convention.
.vortex/tests/bats/unit/deploy-lagoon.bats (10)
46-47: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
79-80: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
115-116: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
160-161: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
195-196: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
236-237: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
283-284: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
314-315: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
349-350: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
384-385: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
.vortex/tests/bats/unit/login-container-registry.bats (1)
36-36: LGTM!Docker config fixture function renamed consistently with the new fixture-based convention.
.vortex/tests/bats/unit/deploy-artifact.bats (2)
26-26: LGTM!Minor text improvement for clarity.
75-77: LGTM!Fixture functions renamed consistently with the new fixture-based convention. The new names better reflect what each function does.
.vortex/tests/bats/unit/setup-ssh.bats (11)
17-17: LGTM!Fixture function renamed consistently with the new fixture-based convention.
31-31: LGTM!Fixture function renamed consistently with the new fixture-based convention.
47-47: LGTM!Fixture function renamed consistently with the new fixture-based convention.
66-67: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
97-99: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
130-130: LGTM!Fixture function renamed consistently with the new fixture-based convention.
152-155: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
186-191: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
226-231: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
267-269: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
303-305: LGTM!Fixture functions renamed consistently with the new fixture-based convention.
.vortex/tests/bats/unit/deploy-container-registry.bats (2)
45-45: LGTM!Docker config fixture function renamed consistently with the new fixture-based convention.
99-99: LGTM!Docker config fixture function renamed consistently with the new fixture-based convention.
| setup_robo_fixture() { | ||
| export HOME="${BUILD_DIR}" | ||
| fixture_prepare_dir "${HOME}/.composer/vendor/bin" | ||
| touch "${HOME}/.composer/vendor/bin/robo" | ||
| chmod +x "${HOME}/.composer/vendor/bin/robo" | ||
|
|
||
| # Also create a mock for git-artifact | ||
| touch "${HOME}/.composer/vendor/bin/git-artifact" | ||
| chmod +x "${HOME}/.composer/vendor/bin/git-artifact" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicated fixture function.
This setup_robo_fixture() function duplicates fixture_robo() already defined in .vortex/tests/bats/_helper.bash (lines 271-280). The implementation is identical. For consistency with the fixture-based refactor across the test suite, use the centralized fixture_robo() function instead.
Apply this diff to remove the duplication:
-setup_robo_fixture() {
- export HOME="${BUILD_DIR}"
- fixture_prepare_dir "${HOME}/.composer/vendor/bin"
- touch "${HOME}/.composer/vendor/bin/robo"
- chmod +x "${HOME}/.composer/vendor/bin/robo"
-
- # Also create a mock for git-artifact
- touch "${HOME}/.composer/vendor/bin/git-artifact"
- chmod +x "${HOME}/.composer/vendor/bin/git-artifact"
-}Then call fixture_robo where needed (though I don't see it being called in the provided code segments, so you may need to verify if it's used elsewhere in this file).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.vortex/tests/bats/unit/deploy-container-registry.bats lines 8-17: the
setup_robo_fixture() function duplicates fixture_robo() defined in
.vortex/tests/bats/_helper.bash (lines 271-280); remove the entire
setup_robo_fixture() function from this file and replace any local calls to it
with a call to fixture_robo(), and then run a quick search in this test file to
ensure no other references remain (if none, no replacement is needed) so the
shared fixture from _helper.bash is used instead.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2076 +/- ##
============================================
+ Coverage 51.87% 67.41% +15.54%
============================================
Files 97 94 -3
Lines 5993 4610 -1383
Branches 44 44
============================================
- Hits 3109 3108 -1
+ Misses 2884 1502 -1382 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #1560
Summary by CodeRabbit