-
Notifications
You must be signed in to change notification settings - Fork 9
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
Added Automation for Bulk Assessment #1642
Conversation
shubham-yb
commented
Aug 21, 2024
•
edited
Loading
edited
- Added the automation script for Bulk Assessment that does the following:
- Create 2 databases on source
- Run the bulk assessment command for 1 correct and 1 incorrect config
- Check for expected output and files
- Fix the incorrect config
- Re-Run the assessment
- Check for expected output and files
- Use the 'move_tables' function to make all the tables sharded for 1 schema
- Run export schema for both
- Check if the recommendations were applied or not
- Added the test corresponding to the script including SSL and prioritisation cases
- Few changes in existing functions
export BULK_ASSESSMENT_DIR=${BULK_ASSESSMENT_DIR:-"${TEST_DIR}/bulk-assessment-dir"} | ||
export SOURCE_DB_SCHEMA=${SOURCE_DB_SCHEMA:-"TEST_SCHEMA"} | ||
export SOURCE_DB_SCHEMA2=${SOURCE_DB_SCHEMA2:-"TEST_SCHEMA2"} | ||
export EXPORT_DIR1=${EXPORT_DIR1:-"${BULK_ASSESSMENT_DIR}/ORCLPDB1-TEST_SCHEMA-export-dir"} |
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.
you can use the above variables here:
export EXPORT_DIR1=${EXPORT_DIR1:-"${BULK_ASSESSMENT_DIR}/${SOURCE_DB_NAME}-${SOURCE_DB_SCHEMA}-export-dir"}
that way if the other variable is changes, this will automatically get updated.
"Change at once place in code, reflected at all"
step "Fix faulty parameter" | ||
fix_config_file fleet-config-file.csv |
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.
Here the fleet file is created once, and we are doing modifications over that using hardcoded shell script code.
Instead of that how about defined the fleet_config_file at the runtime and using that.
With that approach, you can easily write multiple tests for the bulk_assessment with just writing few lines of code in this script(maybe define a function for that)
Also this will make the code more readable.
# Define the fleet config file content
fleet_config_content="source-db-type,source-db-host,source-db-port,source-db-name,oracle-db-sid,oracle-tns-alias,source-db-user,source-db-password,source-db-schema
oracle,localhost,1521,orcl,,ORCLPDB,user1,password1,schema1
oracle,db.example.com,1521,,ORCL,sid1,user2,password2,schema2"
# Create a temporary fleet config file
fleet_config_file=$(mktemp)
# Write the content to the temporary file
echo "$fleet_config_content" > "$fleet_config_file"
assess-migration-bulk --fleet-config-file "$fleet_config_file" --bulk-assessment-dir "/path/to/bulk-assessment-dir"
rm "$fleet_config_file"
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.
Discussed with @sanyamsinghal .
Not required for now since it will bind the test script to a single config.
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.
Please update the PR with details about what test added, and same in the final commit message before merge.
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.
LGTM, with few comments
fix_config_file() { | ||
local file="$1" | ||
awk -F, 'NR==3 {$8="password"}1' OFS=, "$file" > tmp && mv tmp "$file" | ||
} |
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.
Won't this be a test a specific change/fix?
can we have a script in each TEST_DIR for fixing -fix_fleet_config.sh
and use that
while [ $# -gt 0 ]; do | ||
case "$1" in | ||
export_dir) | ||
export_dir="$2" | ||
shift 2 | ||
;; | ||
source_db_schema) | ||
source_db_schema="$2" | ||
shift 2 | ||
;; | ||
*) | ||
break | ||
;; | ||
esac | ||
done |
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.
this is new addition, i hope you have verified other run scripts also which use export_schema command
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.
Yes all the other automation tests are passing.
export_schema() { | ||
args="--export-dir ${EXPORT_DIR} | ||
--source-db-type ${SOURCE_DB_TYPE} | ||
--source-db-user ${SOURCE_DB_USER} | ||
--source-db-password ${SOURCE_DB_PASSWORD} | ||
# Default values | ||
export_dir="${EXPORT_DIR}" | ||
source_db_schema="${SOURCE_DB_SCHEMA}" |
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.
add a comment before the function, explain how are the args passed to it, i.e. [args_name1 arg_val1...]