Skip to content

Added timing to the provision command. #1850

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.

Sign up for GitHub

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

Merged
AlexSkrypnyk merged 1 commit into from
Jul 26, 2025
Merged

Added timing to the provision command. #1850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .vortex/tests/bats/unit/provision.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)
Comment on lines +167 to 168
Copy link

@coderabbitai coderabbitai bot Jul 26, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Test expectations no longer cover the new “(Xm Ys)” suffix

The assertions were trimmed to "Finished site provisioning" (without the period) but the production script now appends timing info like (0m 03s)..
While substring matching will still pass, we lose coverage that the timing text is present and correctly formatted.

Consider tightening the check, e.g.:

assert_output_contains_regex 'Finished site provisioning \([0-9]\+m [0-9]\+s\)\.'

(or equivalent helper) so the tests fail if the timing output is accidentally removed in the future.

Also applies to: 298-299, 438-439, 583-584, 723-724, 857-858, 998-999, 1117-1118

🤖 Prompt for AI Agents 
In .vortex/tests/bats/unit/provision.bats around lines 167 to 168, the test
assertion only checks for the substring "Finished site provisioning" but does
not verify the appended timing suffix like "(Xm Ys).". Update the assertion to
use a regex that matches the full string including the timing format, for
example using assert_output_contains_regex with a pattern like 'Finished site
provisioning \([0-9]\+m [0-9]\+s\)\.'. Apply similar regex-based assertions to
the other specified line ranges to ensure the timing output is validated in all
relevant tests.


mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -295,7 +295,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -435,7 +435,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -580,7 +580,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -720,7 +720,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -854,7 +854,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -995,7 +995,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

mocks="$(run_steps "setup")"
Expand Down Expand Up @@ -1114,7 +1114,7 @@ assert_provision_info() {
"Disabled maintenance mode."

# Installation completion.
"Finished site provisioning."
"Finished site provisioning"
)

export VORTEX_PROVISION_SANITIZE_DB_PASSWORD="MOCK_DB_SANITIZE_PASSWORD"
Expand Down
8 changes: 6 additions & 2 deletions scripts/vortex/provision.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ yesno() { [ "${1}" = "1" ] && echo "Yes" || echo "No"; }

info "Started site provisioning."

start_time=$(date +%s)

Comment on lines +81 to +82
Copy link

@coderabbitai coderabbitai bot Jul 26, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Leverage built-in SECONDS or an EXIT trap for simpler, fail-safe timing

Storing start_time=$(date +%s) works, but Bash already exposes the SECONDS counter (seconds since shell start). Using it, or a one-liner trap, eliminates an external date call and guarantees the duration is printed even on unexpected exits:

- start_time=$(date +%s)
+trap 'duration=$SECONDS; info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."' EXIT

Benefits:
• No subshell; one less external dependency
• Timing is emitted for every exit path without duplicating code
• Removes the risk of missing the finish log on future early-return branches

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
start_time=$(date +%s)
trap 'duration=$SECONDS; info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."' EXIT
🤖 Prompt for AI Agents 
In scripts/vortex/provision.sh around lines 81 to 82, replace the manual
start_time assignment using date with the built-in Bash SECONDS variable to
track elapsed time. Remove the external date call and instead use SECONDS to
measure duration. Additionally, add an EXIT trap function that prints the
elapsed time on script exit to ensure timing is logged regardless of how the
script terminates, avoiding duplicated timing code and improving reliability.

if [ "${VORTEX_PROVISION_SKIP}" = "1" ]; then
pass "Skipped site provisioning as VORTEX_PROVISION_SKIP is set to 1."
info "Finished site provisioning."
Expand Down Expand Up @@ -240,7 +242,8 @@ echo
if [ "${VORTEX_PROVISION_POST_OPERATIONS_SKIP}" = "1" ]; then
info "Skipped running of post-provision operations as VORTEX_PROVISION_POST_OPERATIONS_SKIP is set to 1."
echo
info "Finished site provisioning."
duration=$(($(date +%s) - start_time))
info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."
exit 0
Comment on lines +245 to 247
Copy link

@coderabbitai coderabbitai bot Jul 26, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Three copies of the same duration maths – extract once

The identical block

duration=$(($(date +%s) - start_time))
info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."

appears twice (lines 245-247 & 330-331) and would need a third copy for the early-return path. Duplicate snippets are error-prone and already violate DRY.

If you don’t adopt the trap solution, at least factor this into a small helper:

print_duration() {
  local duration=$(( $(date +%s) - start_time ))
  info "Finished site provisioning ($((duration/60))m $((duration%60))s)."
}

and call print_duration in all three places.

Also applies to: 330-331

🤖 Prompt for AI Agents 
In scripts/vortex/provision.sh around lines 245-247 and 330-331, the duration
calculation and logging code is duplicated. To fix this, extract the repeated
code into a helper function named print_duration that calculates the duration
and logs the message. Then replace all occurrences of the duplicated code with
calls to this new function, including the early-return path, to adhere to DRY
principles and reduce error-prone duplication.

fi

Expand Down Expand Up @@ -324,4 +327,5 @@ if [ "${VORTEX_PROVISION_USE_MAINTENANCE_MODE}" = "1" ]; then
echo
fi

info "Finished site provisioning."
duration=$(($(date +%s) - start_time))
info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."