-
Notifications
You must be signed in to change notification settings - Fork 11k
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
Scripting & documenting debugging one test without anything else in the loop. #7096
Scripting & documenting debugging one test without anything else in the loop. #7096
Conversation
For a guide aimed at developers who are already working with the codebase, you might consider naming it something like "USAGE.md", "GUIDE.md", or "GETTING_STARTED.md". (As a counterpart, we may need to also create DEVELOPER.md to store specific quick-starts for maintainers who want to directly contribute to this project.) As you have currently written, it reads more like a blogpost (which you should do btw on your personal blog anyway). With some cleanup to formalise it and be more direct and formal, maybe it can be a good user guide. |
Absolutely, you are correct. I decided to extract just the part about how to debug a specific test without anything else in the loop. This was the actual direct part. I also removed most of the personality to keep it pointed & useful instead of educational. I appreciate your feedback. @mofosyne |
Attempted to replicate your steps based on the readme with some difficulties. I think i was getting a bit confused. Maybe it should be a step by step instruction... Also attempted to see if I can grok your script and make it a bit more standalone, give this a shot. Maybe it might be worth making it into a callable bash script. #!/bin/bash
test_suite="test-tokenizer-0"
# Step 1: Prepare the Build Environment
rm -rf build-ci-debug && mkdir build-ci-debug && cd build-ci-debug
cmake -DCMAKE_BUILD_TYPE=Debug -DLLAMA_CUDA=1 -DLLAMA_FATAL_WARNINGS=ON .. && cd ..
# Step 2: Build the Project
make -j
# Step 3: Navigate to Test Directory
# Tip: If you see a warning about missing "cache" during installation. Then install it to save immense amounts of time!
cd tests
# Step 4: Identify Test Command for Debugging
# The output of this command will give you the command & arguments needed to run GDB.
# -R test-tokenizer : looks for all the test files named test-tokenizer* (R=Regex)
# -N : "show-only" disables test execution & shows test commands that you can feed to GDB.
# -V : Verbose Mode
echo "Avaliable Tests"
ctest -R ${test_suite} -V -N | grep "Total Tests:"
# Step 5: Debug the Test
read -p "Enter the number of the test command you want to debug: " test_number
ctest -R ${test_suite} -V -N | grep "${test_number}: Test command:"
test_bin=ctest -R ${test_suite} -V -N | grep "${test_number}: Test command:" | awk '{print $4}'
gguf_path=$(ctest -R ${test_suite} -V -N | grep "${test_number}: Test command:" | awk '{print $5}')
gdb --args $test_bin $gguf_path |
Thanks, I'll try it & I'll make the test file regex a parameter. |
Ok @mofosyne, I took your good idea & turned it into a script that lets you select a test to debug from a list. I outlined how the scripts works & how to use it in the readme. |
Awesome. I've rejigged the script a bit as I was having difficulties getting ctest to show any number. Still cannot, but added a few checks to make it more obvious. I don't think I want to hold this off too long as if it works for you and is possibly just my setup being the issue then we can include it in anyway and see how other developers find the script. One minor thing you may want to see if you can also add (optional) is if you could optionally add
Below is the updated script I've adjusted to add more 'early exit' error checks #!/bin/bash
# Function to select and debug a test
function select_test() {
test_suite="$1"
# Sanity Check If Tests Is Detected
max_tests=($(ctest -R ${test_suite} -V -N | grep "Total Tests:" | cut -d':' -f2 | awk '{$1=$1};1'))
if [ $max_tests -eq "0" ]
then
echo "No tests avaliable"
exit 1
fi
# List out avaliable tests
printf "\n\nGathering tests that fit the REGEX: ${test_suite} ...\n"
printf "Which test would you like to debug?\n"
id=0
tests=($(ctest -R ${test_suite} -V -N | grep "Test.\ #*" | cut -d':' -f2 | awk '{$1=$1};1'))
gdb_params=($(ctest -R test-tokenizer -V -N | grep "Test command" | cut -d':' -f3 | awk '{$1=$1};1'))
for s in "${tests[@]}"
do
echo "Test# ${id}"
echo " $s"
((id++))
done
# Prompt user which test they wanted to run
printf "\nRun test#? "
read n
# Start GDB with the requested test binary and test arguments
printf "Debugging(GDB) test: ${tests[n]} ...\n\n"
test=${gdb_params[n*2]}
test_arg=$(echo ${gdb_params[n*2+1]} | sed -e 's/^.//' -e 's/.$//')
gdb --args ${test} ${test_arg}
}
# Step 0: Check the args
if [ $# -ne 1 ] || [ "$1" = "help" ]
then
echo "Supply one regex to the script, e.g. test-tokenizer would\n"
echo "return all the tests in files that match with test-tokenizer."
exit 1
fi
# Step 1: Prepare the Build Environment
## Sanity check that we are actually in a git repo
repo_root=$(git rev-parse --show-toplevel)
if [ ! -d "$repo_root" ]; then
echo "Error: Not in a Git repository."
exit 1
fi
## Build test binaries
pushd "$repo_root" || exit 1
rm -rf build-ci-debug && mkdir build-ci-debug && pushd build-ci-debug || exit 1
cmake -DCMAKE_BUILD_TYPE=Debug -DLLAMA_CUDA=1 -DLLAMA_FATAL_WARNINGS=ON .. && popd || exit 1
make -j || exit 1
pushd tests || exit 1
# Step 2: Debug the Test
select_test "$1"
# Step 3: Return to the directory from which the user ran the command.
popd || exit 1
popd || exit 1 |
Thanks for the error checking. I'll add that in & I'll do more testing with the other test suites today & update the script with anything I find. I appreciate your help. |
Alright @mofosyne, now it's a lot more robust, I ran it on many different test suites & fixed regex problems that I found. I also made the invocation of gdb capable of taking many arguments in case any of the test suites are setup that way. It seems to work well now in all the cases I tried out. A good way to see that it works across the suite of tests is to run |
Sorry to do this to you. But I think i narrowed what went wrong when trying to replicate your script. Your script as making lots of assumption about the state of the repo. After studying the ci flow in github actions in this repo, I think you are actually meant to run ctest within the build folder you created. Ultimately, this is what appears to work reliably on my setup after rejigging your work further. Also made it such that you can also provide the test number in the commandline as well if you already know what test number you are trying to test against in the test suite. Also updated the help $ ./scripts/debug-test.sh
Usage: ./scripts/debug-test.sh [test_regex] [test_number]
e.g., ./scripts/debug-test.sh test-tokenizer
./scripts/debug-test.sh test-tokenizer 3
Supply one regex to the script to filter tests,
and optionally a test number to run a specific test. When you run this line, it will ask you for the test number (this is same as your existing script intended behavior) ./scripts/debug-test.sh test But this is a new one, where if you add the number it will skip the number prompting behavior. ./scripts/debug-test.sh test 3 Here is the updated script so you can check against your setup and then commit it if it works well. I've tested this to work on my setup. #!/bin/bash
build_dir="build-ci-debug"
test_suite=${1:-}
test_number=${2:-}
# Function to select and debug a test
function select_test() {
test_suite=${1:-test}
test_number=${2:-}
# Sanity Check If Tests Is Detected
printf "\n\nGathering tests that fit REGEX: ${test_suite} ...\n"
tests=($(ctest -R ${test_suite} -V -N | grep -E " +Test +#[0-9]+*" | cut -d':' -f2 | awk '{$1=$1};1'))
if [ ${#tests[@]} -eq 0 ]
then
echo "No tests avaliable... check your compliation process..."
echo "Exiting."
exit 1
fi
if [ -z $test_number ]
then
# List out avaliable tests
printf "Which test would you like to debug?\n"
id=0
for s in "${tests[@]}"
do
echo "Test# ${id}"
echo " $s"
((id++))
done
# Prompt user which test they wanted to run
printf "\nRun test#? "
read test_number
else
printf "\nUser Already Requested #${test_number}"
fi
# Start GDB with the requested test binary and arguments
printf "Debugging(GDB) test: ${tests[test_number]}\n"
# Change IFS (Internal Field Separator)
sIFS=$IFS
IFS=$'\n'
# Get test args
gdb_args=($(ctest -R ${test_suite} -V -N | grep "Test command" | cut -d':' -f3 | awk '{$1=$1};1' ))
IFS=$sIFS
printf "Debug arguments: ${gdb_args[test_number]}\n\n"
# Expand paths if needed
args=()
for x in $(echo ${gdb_args[test_number]} | sed -e 's/"\/\<//' -e 's/\>"//')
do
args+=($(echo $x | sed -e 's/.*\/..\//..\//'))
done
# Execute debugger
echo "gdb args: ${args[@]}"
gdb --args ${args[@]}
}
# Step 0: Check the args
if [ -z "$test_suite" ] || [ "$1" = "help" ]
then
echo "Usage: $0 [test_regex] [test_number]"
echo "e.g., $0 test-tokenizer"
echo " $0 test-tokenizer 3"
echo "Supply one regex to the script to filter tests,"
echo "and optionally a test number to run a specific test."
exit 1
fi
# Step 1: Reset and Setup folder context
## Sanity check that we are actually in a git repo
repo_root=$(git rev-parse --show-toplevel)
if [ ! -d "$repo_root" ]; then
echo "Error: Not in a Git repository."
exit 1
fi
## Reset folder to root context of git repo
pushd "$repo_root" || exit 1
## Create and enter build directory
rm -rf "$build_dir" && mkdir "$build_dir" && pushd "$build_dir" || exit 1
# Step 2: Setup Build Environment and Compile Test Binaries
cmake -DCMAKE_BUILD_TYPE=Debug -DLLAMA_CUDA=1 -DLLAMA_FATAL_WARNINGS=ON .. && make -j || exit 1
# Step 3: Debug the Test
select_test "$test_suite" "$test_number"
# Step 4: Return to the directory from which the user ran the command.
popd || exit 1
popd || exit 1
popd || exit 1 |
Thanks, nice cleanup & additional argument.
Exactly what you provided me fails because I think it relies on CMake having persisted information about where to output builds. I altered what you gave me, just lines 90-91, to do exactly the same thing it did but to use the |
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.
Okay, thanks for your contribution! I've adjusted the help a bit and was able to confirm that you change still works on my side.
I've also taken extra time to resync the documentation to match the new behavior of the script that you pushed in.
I've approved the change as I'm now confident that this script is portable (at least between us two). Also thanks for your suggestion on cleaning up my repo, to make the portability check more robust.
Anyway feel free to push in any more minor changes to the script (or minor/major change to the markdown documentation) if you feel so and merge anytime you are ready.
I don't have write access for the merge. Thanks for working with me, I appreciate you. Looking forward to the next time. |
All good! At least you have now made an easier way for you and others to debug this. Definitely consider making a writeup of your deleted content in your personal blog and point to this PR so people can better recognise your contribution and also discover this option. Aside from that, all the best and will be more than happy to check out your future contributions! |
Here is a little document about how to debug one test without having anything else run to get a short feedback loop.