Skip to content
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

Add test script #33

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add test script #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b56834e
Add test_chkdm.sh test script
PeterDaveHello Jan 12, 2025
d817387
Replace single domain testing with test script on Travis CI
PeterDaveHello Jan 12, 2025
8f36c6f
Improve macOS test script compatibility
PeterDaveHello Jan 13, 2025
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 .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ jobs:
- echo -e '8.8.4.4\n1.0.0.1\n1.1' > CustomDNS.txt
- echo -e '223.5.5.5\n114.114.114.114' > CustomDNS2.txt
script:
- ./chkdm dnslow.me
- CustomDNSFile=./CustomDNS2.txt ./chkdm dnslow.me
- rm CustomDNS.txt && ./chkdm dnslow.me
- CustomDNSFile=./CustomDNS-not-exist.txt ./chkdm dnslow.me
- ./test_chkdm.sh
- CustomDNSFile=./CustomDNS2.txt ./test_chkdm.sh
- rm CustomDNS.txt && ./test_chkdm.sh
- CustomDNSFile=./CustomDNS-not-exist.txt ./test_chkdm.sh

- stage: Ubuntu test
env:
Expand Down Expand Up @@ -64,10 +64,10 @@ jobs:
- echo -e '8.8.4.4\n1.0.0.1\n1.1' > CustomDNS.txt
- echo -e '223.5.5.5\n114.114.114.114' > CustomDNS2.txt
script:
- ./chkdm dnslow.me
- CustomDNSFile=./CustomDNS2.txt ./chkdm dnslow.me
- rm CustomDNS.txt && ./chkdm dnslow.me
- CustomDNSFile=./CustomDNS-not-exist.txt ./chkdm dnslow.me
- ./test_chkdm.sh
- CustomDNSFile=./CustomDNS2.txt ./test_chkdm.sh
- rm CustomDNS.txt && ./test_chkdm.sh
- CustomDNSFile=./CustomDNS-not-exist.txt ./test_chkdm.sh

- stage: macOS test
os: osx
Expand Down
206 changes: 206 additions & 0 deletions test_chkdm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
#!/usr/bin/env bash

SCRIPT_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
CHKDM="${SCRIPT_DIR}/chkdm"

RED='\033[0;31m'
GREEN='\033[0;32m'
BLUE='\033[0;34m'
YELLOW='\033[0;33m'
NC='\033[0m'
Comment on lines +1 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add script safety flags and improve portability.

  1. Add safety flags to catch errors early:
 #!/usr/bin/env bash
+set -euo pipefail
+IFS=$'\n\t'
  1. Make readlink usage portable to macOS:
-SCRIPT_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
📝 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
#!/usr/bin/env bash
SCRIPT_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
CHKDM="${SCRIPT_DIR}/chkdm"
RED='\033[0;31m'
GREEN='\033[0;32m'
BLUE='\033[0;34m'
YELLOW='\033[0;33m'
NC='\033[0m'
#!/usr/bin/env bash
set -euo pipefail
IFS=$'\n\t'
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
CHKDM="${SCRIPT_DIR}/chkdm"
RED='\033[0;31m'
GREEN='\033[0;32m'
BLUE='\033[0;34m'
YELLOW='\033[0;33m'
NC='\033[0m'


get_time_ms() {
if command -v gdate >/dev/null 2>&1; then
gdate +%s%N
elif date --version >/dev/null 2>&1 && date +%N >/dev/null 2>&1; then
date +%s%N
else
echo $(($(date +%s) * 1000000000))
fi
}

format_duration() {
local duration=$1
if (( duration < 1000000 )); then
echo "<1"
else
echo "$((duration / 1000000))"
fi
}

test_single_valid_domain() {
local domain="$1"
local start_time end_time duration
start_time=$(get_time_ms)
local result=0

if ! $CHKDM "$domain" > /dev/null 2>&1; then
echo -e " ${RED}✗ Error: domain '$domain' was marked as invalid${NC}"
result=1
else
echo -e " ${GREEN}✓ OK${NC}"
fi

end_time=$(get_time_ms)
duration=$(( end_time - start_time ))
echo -e " ${BLUE}Time: ${YELLOW}$(format_duration $duration)ms${NC}\n"

return $result
}

test_valid_domains() {
local valid_domains=(
"dnslow.me"
"google.com"
"www.apple.com"
"radar.cloudflare.com"
"support.office.microsoft.com"
"push.notifications.prod.facebook.com"
)

local start_time end_time duration
start_time=$(get_time_ms)
local passed=0
local total=${#valid_domains[@]}

echo -e "${BLUE}Testing valid domains...${NC}"
echo -e "${YELLOW}Total valid test cases: $total${NC}\n"

local tmp_dir
tmp_dir=$(mktemp -d)
trap 'rm -rf "$tmp_dir"' EXIT
Comment on lines +69 to +71
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for temporary directory operations.

The script should handle failures in temporary directory creation and ensure atomic file operations.

-    tmp_dir=$(mktemp -d)
+    tmp_dir=$(mktemp -d) || { echo "Failed to create temp directory"; return 1; }
+    chmod 700 "$tmp_dir"
     trap 'rm -rf "$tmp_dir"' EXIT
📝 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
local tmp_dir
tmp_dir=$(mktemp -d)
trap 'rm -rf "$tmp_dir"' EXIT
local tmp_dir
tmp_dir=$(mktemp -d) || { echo "Failed to create temp directory"; return 1; }
chmod 700 "$tmp_dir"
trap 'rm -rf "$tmp_dir"' EXIT

Copy link
Owner Author

Choose a reason for hiding this comment

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

Which part ensures the atomic file operations, chmod 700?

Copy link
Contributor

Choose a reason for hiding this comment

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

@PeterDaveHello You're right to question this. I apologize for the confusion in my previous comment. The chmod 700 command only sets directory permissions (read/write/execute for the owner only) and doesn't ensure atomic operations. The mktemp -d command does provide safety by atomically creating a unique temporary directory, but I was incorrect to mention "atomic file operations" in the context of these changes. Would you like me to revise my suggestion to focus solely on error handling for temporary directory creation?

✏️ Learnings added 
Learnt from: PeterDaveHello
PR: PeterDaveHello/chkdomain#33
File: test_chkdm.sh:69-71
Timestamp: 2025-01-14T16:42:09.715Z
Learning: The suggestion to ensure "atomic file operations" was incorrect in the context of temporary directory creation. The `chmod 700` command only sets directory permissions (rwx for owner only) and doesn't provide atomicity. The `mktemp -d` command itself provides safety by atomically creating a unique temporary directory.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


for domain in "${valid_domains[@]}"; do
echo -e " Testing domain: ${BLUE}$domain${NC}"
(
if test_single_valid_domain "$domain"; then
echo "1" > "$tmp_dir/$domain"
else
echo "0" > "$tmp_dir/$domain"
fi
) &
done

wait

for domain in "${valid_domains[@]}"; do
if [ -f "$tmp_dir/$domain" ] && [ "$(cat "$tmp_dir/$domain")" = "1" ]; then
((passed++))
fi
done

end_time=$(get_time_ms)
duration=$(( (end_time - start_time) / 1000000 ))
echo -e "${BLUE}Total time elapsed: ${YELLOW}${duration}ms${NC}"
echo -e "${GREEN}✓ Valid domains test completed ($passed/$total)${NC}\n"

[ "$passed" -eq "$total" ] && return 0 || return 1
}

test_invalid_domains() {
local invalid_domains=(
"example.123"
"example.c-m"
"example.a"
"this-is-a-very-long-label-that-exceeds-the-63-character-limit123.com"
"sub.this-is-another-very-long-label-that-exceeds-the-63-character-limit.net"
"example--domain.com"
"sub--domain.net"
""
"test..domain"
"invalid#domain.com"
)

local start_time end_time duration
start_time=$(get_time_ms)
local passed=0
local total=${#invalid_domains[@]}

echo -e "${BLUE}Testing invalid domains...${NC}"
echo -e "${YELLOW}Total invalid test cases: $total${NC}\n"

for domain in "${invalid_domains[@]}"; do
echo -e " Testing domain: ${BLUE}$domain${NC}"
local test_start_time test_end_time test_duration
test_start_time=$(get_time_ms)

if $CHKDM "$domain" > /dev/null 2>&1; then
echo -e " ${RED}✗ Error: domain '$domain' was marked as valid${NC}"
else
echo -e " ${GREEN}✓ OK${NC}"
((passed++))
fi

test_end_time=$(get_time_ms)
test_duration=$(( (test_end_time - test_start_time) / 1000000 ))
echo -e " ${BLUE}Time: ${YELLOW}${test_duration}ms${NC}\n"
done

end_time=$(get_time_ms)
duration=$(( (end_time - start_time) / 1000000 ))
echo -e "${BLUE}Total time elapsed: ${YELLOW}${duration}ms${NC}"
echo -e "${GREEN}✓ Invalid domains test completed ($passed/$total)${NC}\n"

[ "$passed" -eq "$total" ] && return 0 || return 1
}

check_chkdm_executable() {
echo -e " ${BLUE}Checking program: ${NC}"
if [ ! -f "$CHKDM" ]; then
echo -e " ${RED}✗ Error: program not found${NC}"
return 1
fi
echo -e " ${GREEN}✓ Found${NC}"

echo -e " ${BLUE}Checking permissions${NC}"
if [ ! -x "$CHKDM" ]; then
echo -e " ${RED}✗ Error: program is not executable${NC}"
return 1
fi
echo -e " ${GREEN}✓ Executable${NC}\n"
return 0
}
Comment on lines +147 to +162
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add absolute path validation.

Ensure the CHKDM path is absolute to prevent potential security issues.

 check_chkdm_executable() {
+    if [[ "$CHKDM" != /* ]]; then
+        echo -e "    ${RED}✗ Error: program path must be absolute${NC}"
+        return 1
+    fi
+
     echo -e "  ${BLUE}Checking program: ${NC}"
📝 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
check_chkdm_executable() {
echo -e " ${BLUE}Checking program: ${NC}"
if [ ! -f "$CHKDM" ]; then
echo -e " ${RED}✗ Error: program not found${NC}"
return 1
fi
echo -e " ${GREEN}✓ Found${NC}"
echo -e " ${BLUE}Checking permissions${NC}"
if [ ! -x "$CHKDM" ]; then
echo -e " ${RED}✗ Error: program is not executable${NC}"
return 1
fi
echo -e " ${GREEN}✓ Executable${NC}\n"
return 0
}
check_chkdm_executable() {
if [[ "$CHKDM" != /* ]]; then
echo -e " ${RED}✗ Error: program path must be absolute${NC}"
return 1
fi
echo -e " ${BLUE}Checking program: ${NC}"
if [ ! -f "$CHKDM" ]; then
echo -e " ${RED}✗ Error: program not found${NC}"
return 1
fi
echo -e " ${GREEN}✓ Found${NC}"
echo -e " ${BLUE}Checking permissions${NC}"
if [ ! -x "$CHKDM" ]; then
echo -e " ${RED}✗ Error: program is not executable${NC}"
return 1
fi
echo -e " ${GREEN}✓ Executable${NC}\n"
return 0
}


main() {
echo -e "${BLUE}Starting domain validation tests...${NC}\n"
local total_start_time total_end_time total_duration
total_start_time=$(get_time_ms)

check_chkdm_executable || exit 1

test_invalid_domains
invalid_result=$?

test_valid_domains
valid_result=$?

total_end_time=$(get_time_ms)
total_duration=$(( (total_end_time - total_start_time) / 1000000 ))

echo -e "${BLUE}Test Summary:${NC}"
if [ $valid_result -eq 0 ]; then
echo -e " ${GREEN}✓ Valid domains: All tests passed${NC}"
else
echo -e " ${RED}✗ Valid domains: Tests failed${NC}"
fi

if [ $invalid_result -eq 0 ]; then
echo -e " ${GREEN}✓ Invalid domains: All tests passed${NC}"
else
echo -e " ${RED}✗ Invalid domains: Tests failed${NC}"
fi

local total_passed=$(( (valid_result == 0 ? 1 : 0) + (invalid_result == 0 ? 1 : 0) ))
echo -e "\n${BLUE}Final Score: ${YELLOW}($total_passed/2)${NC}"
echo -e "${BLUE}Total time elapsed: ${YELLOW}${total_duration}ms${NC}"

if [ $valid_result -eq 0 ] && [ $invalid_result -eq 0 ]; then
echo -e "\n${GREEN}✓ All tests passed!${NC}"
exit 0
else
echo -e "\n${RED}✗ Tests failed${NC}"
exit 1
fi
}

main