DAT-13177 Advanced Harness Test v2 #497
Conversation
There was a problem hiding this comment.
Hello, @yodzhubeiskyi! Great job, this version of the advanced test is much better than the initial one. I've requested some changes and after you will make them, you can merge this PR.
|
|
||
| and: "configuring test data for generateChangelog command for all files format" | ||
| def shortDbName = getShortDatabaseName(testInput.databaseName) | ||
| def generateChangelogMap = configureChangelogMap(testInput.generateCLResourcesPath, shortDbName) |
There was a problem hiding this comment.
Please rename 'generateCLResourcesPath' to 'generateChangelogResourcesPath' for better understanding.
| and: "configuring test data for generateChangelog command for all files format" | ||
| def shortDbName = getShortDatabaseName(testInput.databaseName) | ||
| def generateChangelogMap = configureChangelogMap(testInput.generateCLResourcesPath, shortDbName) | ||
| def diffChangelogMap = configureChangelogMap(testInput.diffCLResourcesPath, shortDbName) |
|
|
||
| then: "check if a changelog was actually generated and validate it's content" | ||
| String generatedChangelog = parseQuery(readFile((String) argsMapPrimary.get("changelogFile"))) | ||
| validateChangelog(entry.key, generatedChangelog, testInput.verificationGenCLSql, testInput.change, null) |
There was a problem hiding this comment.
Please refactor validateChangelog method, so don't have to use null value here.
| } | ||
|
|
||
| when: "execute diff command" | ||
| String generatedDiff = cleanDiff(executeCommandScope("diff", argsMapSecondary).toString()) |
There was a problem hiding this comment.
Please rename the method 'cleanDiff' to something like 'getCleanDiff', or another name that will better reflect the functionality of this method.
| argsMapSecondary.put("changelogFile", resourcesDirFullPath + entry.value) | ||
| executeCommandScope("diffChangelog", argsMapSecondary) | ||
|
|
||
| then: "check if a changelog was actually generated and validate it's content" |
There was a problem hiding this comment.
It looks to me that it would be better to use something like 'verify diff changelog...'
| return map | ||
| } | ||
|
|
||
| static validateChangelog( String changelogFormat, String changelogContent, String verificationSql, String change, String changeReversed) { |
There was a problem hiding this comment.
Please create two separate methods instead of this one, name the first of them 'validateGeneratedChangelog' and the second one 'validateDiffChangelog'
README.md
Outdated
|
|
||
| ### Configuring Advanced test | ||
| 1) Go to `src/main/resources/liquibase/harness/compatibility/advanced/changelogs` and add the xml changelog for the change type you want to test. | ||
| 1) Go to `src/main/resources/liquibase/harness/compatibility/advanced/initSql/primary` and add sql script for the change type you want to test. | ||
| - Use change type as changelog name (createTable.xml, addCheckConstraint.xml, etc.) as the test will use it for generated changelog validation. | ||
| - Some change types (addColumn, addPrimaryKey, addUniqueConstraint) don't always produce separate changeset during generateChangelog/diffChangelog execution, use column.xml, primary.xml, unique.xml |
There was a problem hiding this comment.
since we now use init.sql scripts instead of XML changelogs this description is not correct.
README.md
Outdated
| - In case expectedSql is not provided, the test will auto-generate one to `src/test/resources/liquibase/harness/compatibility/advanced/expectedSql/generateChangelog` folder. Please verify its content and use it as expectedSql test data. | ||
| 5) Go to `src/main/resources/liquibase/harness/compatibility/advanced/expectedSql/diffChangelog` and add query you expect liquibase to generate during updateSql command execution for generated diff changelog. | ||
| - In case expectedSql is not provided, the test will auto-generate one to `src/test/resources/liquibase/harness/compatibility/advanced/expectedSql/diffChangelog` folder. Please verify its content and use it as expectedSql test data. | ||
| 6) Go to `src/main/resources/liquibase/harness/compatibility/advanced/expectedSnapshot` and add expected DB Snapshot results. |
There was a problem hiding this comment.
Please add a note here regarding the usage of example.snapshot file.
README.md
Outdated
| 6) Go to `src/main/resources/liquibase/harness/compatibility/advanced/expectedDiff` and add expected diff results. | ||
| - You will need to add this under the database specific folder. | ||
| - If you would like to test another DB type, please add the requisite folder. | ||
| 7) Go to `src/main/resources/liquibase/harness/compatibility/advanced/expectedDiff` and add expected diff results. |
There was a problem hiding this comment.
Add the same note regarding the example file
There was a problem hiding this comment.
I made some minor edits @yodzhubeiskyi , but overall this looks very good, thank you for all the work here!
Please also add the examples that @PavloTytarchuk is asking for and I think we should remove references that still exist in the readme about Running Advanced test suite against your database since we now want users to use the new Advanced Test class and not the earlier Test Suite.
No description provided.