Skip to content
Open
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
45 changes: 42 additions & 3 deletions test.ps1
Original file line number Diff line number Diff line change
@@ -1,3 +1,42 @@
cd basic-commits
.\setup.ps1
cd ..
# ****************************************************************************
# Get all the setup.ps1 files and execute them to check if any errors occure
# Created by: Tomi Peltonen 2018-12-10
# ****************************************************************************
try
{
write-host "Executing the exercise setup.ps1 files to make sure they are running properly." -ForegroundColor Black -BackgroundColor Green

git config --global --replace-all user.name "Tomi Peltonen"
git config --global --replace-all user.email "top@praqma.net"

Write-Host "Git user updated" -foregroundcolor blue

Write-Host "user.name: " -foregroundcolor blue -nonewline
git config --global user.name

Write-Host "user.email: " -foregroundcolor blue -nonewline
git config --global user.email
Comment on lines +9 to +18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Global git identity is overwritten with hardcoded values and never restored.

git config --global --replace-all permanently overwrites the developer's/CI machine's actual global user.name/user.email with hardcoded values, with no capture/restore of the previous values in the finally block. Anyone running this test script (e.g., locally) will have their real git identity silently replaced.

🔧 Proposed fix to preserve and restore the original identity
+$prevName = git config --global user.name
+$prevEmail = git config --global user.email
+
 git config --global --replace-all user.name "Tomi Peltonen"
 git config --global --replace-all user.email "top@praqma.net"
 finally
 {
+	git config --global --replace-all user.name "$prevName"
+	git config --global --replace-all user.email "$prevEmail"
 	cd..
 	write-host "Finally block reached, we are good!" -ForegroundColor Black -BackgroundColor Green
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.ps1` around lines 9 - 18, The script is overwriting the machine’s global
git identity with hardcoded values and never restoring it. Update the test
script around the git config setup so it first captures the existing global
user.name and user.email, applies the temporary values only for the test, and
then restores the originals in the cleanup/finally path. Use the existing git
config commands and the surrounding cleanup logic in test.ps1 to ensure the
developer/CI identity is preserved.


$file = Get-ChildItem -Path $scriptPath -Recurse -Include setup.ps1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

$scriptPath is never defined.

Get-ChildItem -Path $scriptPath references a variable that isn't declared or assigned anywhere in this file. This will resolve to $null, causing Get-ChildItem to error or default to an unintended location.

🐛 Proposed fix
+$scriptPath = $PSScriptRoot
+
 $file = Get-ChildItem -Path $scriptPath -Recurse -Include setup.ps1
📝 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
$file = Get-ChildItem -Path $scriptPath -Recurse -Include setup.ps1
$scriptPath = $PSScriptRoot
$file = Get-ChildItem -Path $scriptPath -Recurse -Include setup.ps1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.ps1` at line 20, The setup file lookup in the test script uses an
undefined $scriptPath variable, so fix the path source in the Get-ChildItem call
to use a defined location from this script or initialize $scriptPath before it
is used. Locate the change around the Get-ChildItem invocation in test.ps1 and
ensure the variable is assigned a valid path before the recursive search for
setup.ps1 runs.


for ($i=0; $i -lt $file.length; $i++)
{
Set-Location $file[$i].Directory
Comment on lines +20 to +24

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '--- test.ps1 (numbered) ---\n'
cat -n test.ps1 | sed -n '1,120p'
printf '\n--- grep for $file usage in test.ps1 ---\n'
rg -n '\$file\b|Get-ChildItem -Path \$scriptPath -Recurse -Include setup\.ps1' test.ps1 || true
printf '\n--- repository search for similar patterns ---\n'
rg -n '\$[A-Za-z_][A-Za-z0-9_]*\.length|\[[0-9]+\]' -g '*.ps1' .

Repository: eficode-academy/git-katas

Length of output: 254


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'pwsh path: '
command -v pwsh || true
printf '\n'

if command -v pwsh >/dev/null 2>&1; then
  pwsh -NoLogo -NoProfile -Command '
    $scalar = [System.IO.FileInfo]::new("/tmp/example.txt")
    $arr = @($scalar)

    "scalar type: $($scalar.GetType().FullName)"
    "scalar Length property: $($scalar.Length)"
    "scalar[0] type: $($scalar[0].GetType().FullName)"
    "scalar[1] is null: $($null -eq $scalar[1])"
    "array length: $($arr.Length)"
  '
fi

printf '\n--- test.ps1 ---\n'
cat -n test.ps1 | sed -n '1,80p'

Repository: eficode-academy/git-katas

Length of output: 2042


Wrap Get-ChildItem in @(...) here
When only one setup.ps1 exists, $file is a FileInfo, so .Length is the file size and the loop can iterate the wrong number of times. @(...) keeps $file as a collection and makes $file[$i] safe.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.ps1` around lines 20 - 24, Wrap the Get-ChildItem result in @(...) in
the setup.ps1 discovery logic so $file is always an array-like collection
instead of a single FileInfo object. Update the test.ps1 loop that uses
$file.length and $file[$i] so it works reliably in the search-and-iterate block
even when only one setup.ps1 is found.

Write-Host Directory: $file[$i].Directory -ForegroundColor Black -BackgroundColor Yellow

.\setup.ps1

cd..
}
Comment on lines +22 to +30

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Loop always ascends exactly one directory level regardless of nesting depth.

Since setup.ps1 files are found recursively (potentially at varying depths), returning with a single cd.. after each iteration only correctly restores the working directory when every match is exactly one level below $scriptPath. If any exercise nests setup.ps1 deeper, subsequent iterations use a stale directory as their base for Set-Location.

Consider setting the location back to $scriptPath directly after each script invocation instead of relying on relative cd.., as shown in the previous suggested diff.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.ps1` around lines 22 - 30, The loop in test.ps1 restores the working
directory with a fixed one-level ascent, which breaks when setup.ps1 is found at
different nesting depths. Update the iteration around Set-Location and
.\setup.ps1 so it always returns to the original $scriptPath after each
invocation instead of using relative cd..; this keeps each pass independent
regardless of where the matching setup.ps1 was discovered.

}
Comment on lines +5 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Wrapping the entire discovery loop in one try defeats the script's stated purpose.

The header comment states the goal is to "execute them to check if any errors occure" across all exercises, but since the whole for loop sits inside a single try, the first setup.ps1 that throws aborts execution of all remaining setup scripts — only one failure is ever reported per run.

♻️ Proposed fix: per-file error handling
 for ($i=0; $i -lt $file.length; $i++) 
 {
-	Set-Location $file[$i].Directory
-	Write-Host Directory: $file[$i].Directory -ForegroundColor Black -BackgroundColor Yellow
+	try
+	{
+		Set-Location $file[$i].Directory
+		Write-Host Directory: $file[$i].Directory -ForegroundColor Black -BackgroundColor Yellow
+		.\setup.ps1
+	}
+	catch
+	{
+		write-host "Error running $($file[$i].FullName): $($_.Exception.Message)" -ForegroundColor Red
+	}
+	finally
+	{
+		Set-Location $scriptPath
+	}
+}
-
-	.\setup.ps1
-
-	cd..
-}	
📝 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
try
{
write-host "Executing the exercise setup.ps1 files to make sure they are running properly." -ForegroundColor Black -BackgroundColor Green
git config --global --replace-all user.name "Tomi Peltonen"
git config --global --replace-all user.email "top@praqma.net"
Write-Host "Git user updated" -foregroundcolor blue
Write-Host "user.name: " -foregroundcolor blue -nonewline
git config --global user.name
Write-Host "user.email: " -foregroundcolor blue -nonewline
git config --global user.email
$file = Get-ChildItem -Path $scriptPath -Recurse -Include setup.ps1
for ($i=0; $i -lt $file.length; $i++)
{
Set-Location $file[$i].Directory
Write-Host Directory: $file[$i].Directory -ForegroundColor Black -BackgroundColor Yellow
.\setup.ps1
cd..
}
}
try
{
write-host "Executing the exercise setup.ps1 files to make sure they are running properly." -ForegroundColor Black -BackgroundColor Green
git config --global --replace-all user.name "Tomi Peltonen"
git config --global --replace-all user.email "top@praqma.net"
Write-Host "Git user updated" -foregroundcolor blue
Write-Host "user.name: " -foregroundcolor blue -nonewline
git config --global user.name
Write-Host "user.email: " -foregroundcolor blue -nonewline
git config --global user.email
$file = Get-ChildItem -Path $scriptPath -Recurse -Include setup.ps1
for ($i=0; $i -lt $file.length; $i++)
{
try
{
Set-Location $file[$i].Directory
Write-Host Directory: $file[$i].Directory -ForegroundColor Black -BackgroundColor Yellow
.\setup.ps1
}
catch
{
write-host "Error running $($file[$i].FullName): $($_.Exception.Message)" -ForegroundColor Red
}
finally
{
Set-Location $scriptPath
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.ps1` around lines 5 - 31, The discovery loop in test.ps1 is wrapped in a
single try block, so one failing setup.ps1 stops the rest from running. Move
error handling into the per-file execution path around the .\setup.ps1 call
inside the for loop (or otherwise continue after each failure) so every
discovered setup script is attempted independently while preserving the loop in
test.ps1.

catch
{
write-host "Caught an exception:" -ForegroundColor Red
write-host "Exception Type: $($_.Exception.GetType().FullName)" -ForegroundColor Red
write-host "Exception Message: $($_.Exception.Message)" -ForegroundColor Red
}
finally
{
cd..
write-host "Finally block reached, we are good!" -ForegroundColor Black -BackgroundColor Green
}
45 changes: 45 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# ****************************************************************************
# Get all the setup.sh files and execute them to check if any errors occure
# Created by:
# ****************************************************************************

cd basic-commits
./setup.sh
cd ..
Comment on lines +6 to +8

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add cd failure guards per Shellcheck.

cd basic-commits at Line 6 can fail silently (e.g., missing/renamed directory) and the script would continue executing ./setup.sh from the wrong directory. Shellcheck SC2164 flags this.

🐛 Proposed fix
-cd basic-commits
-./setup.sh
-cd ..
+cd basic-commits || exit 1
+./setup.sh
+cd .. || exit 1
📝 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
cd basic-commits
./setup.sh
cd ..
cd basic-commits || exit 1
./setup.sh
cd .. || exit 1
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 6-6: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[info] 8-8: Use a ( subshell ) to avoid having to cd back.

(SC2103)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.sh` around lines 6 - 8, The test.sh setup flow does not guard the cd
into basic-commits, so the script can continue in the wrong directory if that
change fails. Update the setup sequence to fail fast on the directory change by
adding an explicit cd failure guard before running ./setup.sh, using the
existing basic-commits cd step as the place to fix this Shellcheck SC2164 issue.

Source: Linters/SAST tools


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== files ==\n'
git ls-files | rg '(^|/)(test\.sh|test\.ps1|setup\.sh|README\.md)$'

printf '\n== test.sh ==\n'
cat -n test.sh

printf '\n== test.ps1 ==\n'
cat -n test.ps1

printf '\n== setup.sh files ==\n'
find . -type f -name setup.sh | sort

printf '\n== mention of "Get all the setup.sh files" ==\n'
rg -n "Get all the setup\.sh files|setup\.sh" -S .

Repository: eficode-academy/git-katas

Length of output: 7911


Run all kata setup scripts, not just basic-commits test.sh:6-8 only executes basic-commits/setup.sh, so the rest of the katas' setup.sh files never get exercised. test.ps1 already discovers them recursively, so the shell version should match that behavior.

🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 6-6: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

(SC2164)


[info] 8-8: Use a ( subshell ) to avoid having to cd back.

(SC2103)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test.sh` around lines 6 - 8, The shell test script is only running the
basic-commits kata setup, so the other kata setup scripts are skipped. Update
the test.sh flow to mirror test.ps1 by discovering and executing every setup.sh
recursively, using the same overall setup routine but without hardcoding
basic-commits. Reference the existing test.sh execution block and the setup.sh
invocation logic when making the change.


#
# try
# {
# echo Executing the exercise setup.ps1 files to make sure they are running properly.

# git config --global --replace-all user.name "Tomi Peltonen"
# git config --global --replace-all user.email "top@praqma.net"

# echo Git user updated

# echo user.name: git config --global user.name

# echo user.email: git config --global user.email

# find . -type f -name setup.sh

# for ($i=0; $i -lt $file.length; $i++)
# {
# Set-Location $file[$i].Directory
# echo Directory: $file[$i].Directory -ForegroundColor Black -BackgroundColor Yellow

# ./setup.sh

# cd ..
# }
# }
# catch
# {
# echo Caught an exception:
# echo "Error in $__EXCEPTION_SOURCE__ at line: $__EXCEPTION_LINE__!"
# }
# finally
# {
# cd ..
# echo Finally block reached, we are good
# }