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.

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

improved uuid checks on commit hook and make targets #3855

Merged
merged 1 commit into from
Sep 27, 2016
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
29 changes: 13 additions & 16 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ help:
@echo " logs-join to join xdist log files into one"
@echo " logs-clean to delete all xdist log files in the root"
@echo " pyc-clean to delete all temporary artifacts"
@echo " uuid-check to check for duplicated @id: in testimony docstring tags"
@echo " uuid-replace-empty to replace empty @id: with new generated uuid"
@echo " uuid-replace-duplicate to replace duplicated @id: with new generated uuid"
@echo " uuid-check to check for duplicated or empty @id: in testimony docstring tags"
@echo " uuid-fix to fix all duplicated or empty @id: in testimony docstring tags"
@echo " can-i-push? to check if local changes are suitable to push"
@echo " install-commit-hook to install pre-commit hook to check if changes are suitable to push"
@echo " gitflake8 to check flake8 styling only for modified files"

docs:
@cd docs; $(MAKE) html
Expand Down Expand Up @@ -132,26 +132,24 @@ logs-join:
logs-clean:
-rm -f robottelo_gw*.log

uuid-check: ## list duplicated uuids
uuid-check: ## list duplicated or empty uuids
$(info "Checking for empty or duplicated @id: in docstrings...")
scripts/check_duplicate_uuids.sh
@scripts/fix_uuids.sh --check

uuid-replace-duplicate: ## list duplicated uuids
scripts/replace_dup_uuids.sh
uuid-fix:
@scripts/fix_uuids.sh

uuid-replace-empty: ## list duplicated uuids
scripts/replace_empty_uuids.sh

flake8:
gitflake8:
$(info "Checking style and syntax errors with flake8 linter...")
@flake8 . --show-source
@flake8 $(shell git diff --name-only) --show-source
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good change, but I feel that it is more suitable for travis and commit hook other than checking local changes, I mean I will not be able to run make flake8 for files that I have not added to git yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elyezer should I change the target name to gitflake or add a new target?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, that is a personal preference but if others don't worry about it then it is fine to leave instead of creating a separated target.

I've just pointed some of the usecase I personally have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to gitflake8


can-i-push?: flake8 uuid-check test-docstrings test-robottelo
can-i-push?: gitflake8 uuid-check test-docstrings test-robottelo
$(info "!!! Congratulations your changes are good to fly, make a great PR! ${USER}++ !!!")

install-commit-hook:
$(info "Installing git pre-commit hook...")
echo "make can-i-push?" >> .git/hooks/pre-commit
@grep -q '^make uuid-fix' .git/hooks/pre-commit || echo "make uuid-fix" >> .git/hooks/pre-commit
@grep -q '^make can-i-push?' .git/hooks/pre-commit || echo "make can-i-push?" >> .git/hooks/pre-commit

# Special Targets -------------------------------------------------------------

Expand All @@ -161,5 +159,4 @@ install-commit-hook:
test-foreman-tier2 test-foreman-tier3 test-foreman-tier4 \
test-foreman-ui test-foreman-ui-xvfb test-foreman-endtoend \
graph-entities lint logs-join logs-clean pyc-clean \
uuid-check uuid-replace-duplicate uuid-replace-empty \
can-i-push? install-commit-hook
uuid-check uuid-fix can-i-push? install-commit-hook gitflake8
27 changes: 0 additions & 27 deletions scripts/check_duplicate_uuids.sh

This file was deleted.

107 changes: 107 additions & 0 deletions scripts/fix_uuids.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
#!/usr/bin/env bash

# This script finds empty @id: and replace with new uuids
# Then it finds for duplicates and replaces last occurrence with new uuids
# finally it fixes '@id:xyz' to '@id: xyz' (missing space after :)

ID_TOKEN="@id:"

if [ "$1" = "--check" ]; then
CHECK_ONLY=true
echo "Running in uuid-check only mode..."
else
CHECK_ONLY=false
echo "Running in uuid-fix mode..."
fi

# finds occurrences of empty id: testimony tags
EMPTY_IDS=$(grep -E -i -r -n "${ID_TOKEN}(.+[[:blank:]]|$)" tests/foreman/ --include=*.py)

if [ -n "$EMPTY_IDS" ]; then
if [ $CHECK_ONLY = true ]; then
echo "Empty $ID_TOKEN found in testimony tags"
echo $EMPTY_IDS
exit 1
else
echo "Generating new UUIDS for empty $ID_TOKEN tags..."
fi
else
echo "No empty $ID_TOKEN was found"
fi

# iterate if any empty @id found
for output_line in $EMPTY_IDS
do
if (echo "$output_line" | grep "tests/foreman"); then
Copy link
Member

Choose a reason for hiding this comment

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

Is this check required? Your grep is already pointing at tests/foreman so I expect that all lines would have this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to include it here because in some cases grep output have been wrapped, but I can try different approach.

OLDIFS=$IFS
# splits the grep output to get filename and occurrence line number
IFS=':' read -r filename line <<< $output_line
# generate uuid and place in specific line number
NEW_ID=$(python -c 'import uuid; print(uuid.uuid4())')
sed -r -i~ "${line}s/${ID_TOKEN}(.+[[:blank:]]|$)/${ID_TOKEN} ${NEW_ID}/g" $filename
IFS=$OLDIFS
fi
done

# This script finds duplicated @id and replaces with new uuids

# Finds occurrences of @id: in testimony tags then
# sort the output and filters only the duplicated
# then looks for existence of "@id:" in final output
# NOTE: can't print the line number -n here because of uniq -d
DUP_EXISTS=$(grep -r -i $ID_TOKEN tests/foreman/ --include=*.py | sort | uniq -d | grep $ID_TOKEN)

if [ -n "$DUP_EXISTS" ]; then
if [ $CHECK_ONLY = true ]; then
echo "Duplicate $ID_TOKEN found in testimony tags"
echo $DUP_EXISTS
exit 1
else
echo "Generating new UUIDS for duplicated $ID_TOKEN tags..."
fi
else
echo "No duplicated $ID_TOKEN was found"
fi

grep -r -i $ID_TOKEN tests/foreman/ --include=*.py | sort | uniq -d | grep $ID_TOKEN | while read -r line ; do
OLDIFS=$IFS
IFS=':' read -r dup_file dup_id <<< $line
echo "filename: $dup_file"
echo "Id to replace: $dup_id"
NEW_ID=$(python -c 'import uuid; print(uuid.uuid4())')
echo "Replacing with the new id: $NEW_ID"
LAST_LINE=$(grep -i -n "$dup_id" $dup_file | tail -1)

IFS=':' read -r linenumber linecontent <<< $LAST_LINE
echo $linenumber
trimmed_linecontent=$(echo $linecontent)
sed -i~ "${linenumber}s/${trimmed_linecontent}/${ID_TOKEN} ${NEW_ID}/g" $dup_file
echo "----------------------------------------------------------------"
IFS=$OLDIFS
done

# This script finds id: missing spaces after :


MISSING_SPACES=$(grep -E -i -r -n "${ID_TOKEN}[^ ]" tests/foreman/ --include=*.py)

if [ -n "$MISSING_SPACES" ]; then
if [ $CHECK_ONLY = true ]; then
echo "Found $ID_TOKEN tags missing space after : ..."
echo $MISSING_SPACES
exit 1
else
echo "Fixing $ID_TOKEN tags missing spaces..."
fi
else
echo "No $ID_TOKEN missing spaces was found"
fi

grep -E -i -r -n "${ID_TOKEN}[^ ]" tests/foreman/ --include=*.py | while read -r line ; do
OLDIFS=$IFS
IFS=':' read -r missing_file linenumber linecontent <<< $line
IFS=':' read -r tag uuid <<< $linecontent
trimmed_linecontent=$(echo $linecontent)
sed -i~ "${linenumber}s/${trimmed_linecontent}/${tag}: ${uuid}/g" $missing_file
IFS=$OLDIFS
done
20 changes: 0 additions & 20 deletions scripts/replace_dup_uuids.sh

This file was deleted.

25 changes: 0 additions & 25 deletions scripts/replace_empty_uuids.sh

This file was deleted.