Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 35 additions & 0 deletions spec/activate_roborev_spec.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
Describe 'home-manager/services/roborev/activate.sh'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing standard spec header: every other spec/activate_*_spec.sh file (activate_qmd_spec.sh, activate_paperclip_openclaw_spec.sh, activate_k3s_spec.sh, activate_neovim_spec.sh, activate_tailscale_spec.sh) begins with:

#!/usr/bin/env bash
# shellcheck disable=SC2016,SC2329

This file starts directly with Describe, which is inconsistent with the established convention and may surface SC2016/SC2329 warnings on the inline single-quoted bash -c "grep '...' '$SCRIPT'" patterns when shellcheck runs over the spec directory.

Suggested change
Describe 'home-manager/services/roborev/activate.sh'
#!/usr/bin/env bash
# shellcheck disable=SC2016,SC2329
Describe 'home-manager/services/roborev/activate.sh'

SCRIPT="$PWD/home-manager/services/roborev/activate.sh"

It 'uses bash shebang'
When run bash -c "head -1 '$SCRIPT'"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The use of bash -c inside When run is unnecessary for simple commands like head or grep. ShellSpec can execute these directly, which is cleaner and more efficient. Additionally, using head -n 1 is more standard than head -1. This pattern should be applied to lines 10, 15, 22, and 27 as well.

Suggested change
When run bash -c "head -1 '$SCRIPT'"
When run head -n 1 "$SCRIPT"

The output should include '#!/usr/bin/env bash'
End

It 'creates the data directory'
When run bash -c "grep 'mkdir' '$SCRIPT'"
The output should include 'mkdir -p'
End

It 'sets restrictive permissions on data directory'
When run bash -c "grep 'chmod' '$SCRIPT'"
The output should include 'chmod 700'
End
End

Describe 'home-manager/services/roborev/default.nix'
It 'enables on galactica and matic only'
When run bash -c "grep 'isGalactica || isMatic' '$PWD/home-manager/services/roborev/default.nix'"
The output should include 'isGalactica || isMatic'
End

It 'runs roborev daemon run'
When run bash -c "grep 'daemon' '$PWD/home-manager/services/roborev/default.nix'"
The output should include '"daemon"'
End

It 'passes data dir to activate script'
When run cat "$PWD/home-manager/services/roborev/default.nix"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using cat to read an entire file just to check for a specific string is less efficient than using grep directly. It is better to use grep to search for the pattern, which is also more consistent with the other tests in this file.

Suggested change
When run cat "$PWD/home-manager/services/roborev/default.nix"
When run grep '"${./activate.sh}" "${dataDir}"' "$PWD/home-manager/services/roborev/default.nix"
References
  1. Maintain consistency with established patterns for writing scripts that are extracted from Nix expressions.

The output should include '"${./activate.sh}" "${dataDir}"'
End
End
1 change: 1 addition & 0 deletions spec/coverage_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ home-manager/services/obsidian/obsidian-headless.sh
home-manager/services/openclaw/activate.sh
home-manager/services/paperclip/activate.sh
home-manager/services/qmd/activate.sh
home-manager/services/roborev/activate.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

While the script is correctly added to the covered_scripts list here, it is missing a corresponding It block in the first Describe section of this file (around line 206) to verify that the spec file itself exists. Following the established pattern in this repository ensures that new spec files are tracked and not accidentally deleted.

home-manager/modules/tailscale/activate-create-dirs.sh
home-manager/modules/tailscale/activate-install-service.sh
home-manager/modules/uv-globals/install-uv-globals.sh
Expand Down
Loading