e2e Tests for cloudSQL-MySQL #7
Conversation
| Then Navigate to the properties page of plugin: "CloudSQL MySQL" | ||
| Then Select dropdown plugin property: "select-jdbcPluginName" with option value: "cloudsql-mysql" | ||
| Then Select radio button plugin property: "instanceType" with value: "public" | ||
| Then Enter input plugin property: "connectionName" with value: "cdf-athena:us-central1:sql-automation-test-instance" |
There was a problem hiding this comment.
cdf-athena:us-central1:sql-automation-test-instance These values shouldn't be hardcoded use them from PluginParameter.properties file. (Update all the test cases.)
| @CloudMySql | ||
| Feature: CloudMySql sink- Verify CloudMySql sink plugin design time scenarios | ||
|
|
||
| Scenario: To verify CloudMySql sink plugin validation with mandatory properties |
There was a problem hiding this comment.
There are more scenarios in the RTM where we are validating the plugin with different properties present in the plugin. Create more Design Time scenarios .
There was a problem hiding this comment.
I don't see this one addressed.
| Then Navigate to the properties page of plugin: "BigQuery" | ||
| Then Replace input plugin property: "project" with value: "projectId" | ||
| Then Enter input plugin property: "datasetProject" with value: "projectId" | ||
| Then Override Service account details if set in environment variables |
There was a problem hiding this comment.
No need to use this step, remove wherever used.
| Then Click on the Macro button of Property: "user" and set the value to: "username" | ||
| Then Click on the Macro button of Property: "password" and set the value to: "password" | ||
| Then Select radio button plugin property: "instanceType" with value: "public" | ||
| Then Enter input plugin property: "connectionName" with value: "cdf-athena:us-central1:sql-automation-test-instance" |
There was a problem hiding this comment.
Same comment don't hardcode this value, use PluginParameter.properties file.
# Conflicts: # cloudsql-mysql-plugin/src/e2e-test/java/io/cdap/plugin/CloudMySqlClient.java # cloudsql-mysql-plugin/src/e2e-test/resources/pluginParameters.properties
| @@ -0,0 +1,4 @@ | |||
| package io.cdap.plugin.CloudMySql.runners; | |||
|
|
|||
There was a problem hiding this comment.
If you are creating this class use it properly define the cucumber options and use the tag in the tests which can be run as required tests.
There was a problem hiding this comment.
Add the link of commit whenever you mark a comment as resolved.
| private static final String database = PluginPropertyUtils.pluginProp("DatabaseName"); | ||
| private static final String connectionName = PluginPropertyUtils.pluginProp("ConnectionName"); | ||
|
|
||
| public static void main(String[] args) throws SQLException, ClassNotFoundException { |
There was a problem hiding this comment.
Remove the main method from here we don't need this in production code.
| String instanceConnectionName = "cdf-athena:us-central1:sql-automation-test-instance"; | ||
| String databaseName = "TestDatabase"; | ||
| String Username = "v"; | ||
| String Password = "v@123"; |
There was a problem hiding this comment.
Use all these credentials by creating environment variables
| String Username = "v"; | ||
| String Password = "v@123"; | ||
| String jdbcUrl = String.format("jdbc:mysql:///%s?cloudSqlInstance=%s&socketFactory=com.google.cloud.sql.mysql.SocketFactory&user=%s&password=%s", databaseName, instanceConnectionName, Username, Password); | ||
| Connection conn = DriverManager.getConnection(jdbcUrl); |
There was a problem hiding this comment.
Don't hardcode this string here. Add it in the plugin parameter properties file and pick it from there.
| String Password = "v@123"; | ||
| String jdbcUrl = String.format("jdbc:mysql:///%s?cloudSqlInstance=%s&socketFactory=com.google.cloud.sql.mysql.SocketFactory&user=%s&password=%s", databaseName, instanceConnectionName, Username, Password); | ||
| Connection conn = DriverManager.getConnection(jdbcUrl); | ||
| System.out.println("connected to database"); |
There was a problem hiding this comment.
remove system.out
| return conn; | ||
| } | ||
|
|
||
| public static int countRecord(String table) throws SQLException, ClassNotFoundException { |
There was a problem hiding this comment.
Add java doc to all the newly created methods defining what they are doing.
| */ | ||
|
|
||
| public class TestSetupHooks { | ||
| public static void main(String[] args) throws SQLException, ClassNotFoundException { |
There was a problem hiding this comment.
Remove main method.
| Then Validate "CloudSQL MySQL" plugin properties | ||
| Then Close the Plugin Properties page | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove the extra spaces just leave one line at last.
| Then Enter input plugin property: "referenceName" with value: "RefName" | ||
| Then Enter input plugin property: "database" with value: "TestDatabase" | ||
| Then Enter textarea plugin property: "importQuery" with value: "insertQuery" | ||
| # Then Click on the Get Schema button |
There was a problem hiding this comment.
Uncomment all the steps.
| Feature: CloudMySql Sink - Run time scenarios | ||
|
|
||
| @BQ_SOURCE_TEST @CLOUDMYSQL_TEST_TABLE | ||
| Scenario: To verify data is getting transferred from BigQuery source to CloudMySql sink successfully |
There was a problem hiding this comment.
Add more run time test scenarios
| Then Open and capture logs | ||
| Then Verify the pipeline status is "Succeeded" | ||
| Then Close the pipeline logs | ||
|
|
There was a problem hiding this comment.
Create a scenario to use macro argument in Advanced section
| Then Close the pipeline logs | ||
|
|
||
| @BQ_SOURCE_TEST @CLOUDMYSQL_TEST_TABLE | ||
| Scenario: Verify pipeline failure message in logs when user provides invalid Table Name of plugin with Macros |
There was a problem hiding this comment.
For these negative tests add step to validate error message on log level.
| Then Click on the Validate button | ||
| Then Close the Plugin Properties page | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove extra lines.
| Then Verify the pipeline status is "Succeeded" | ||
|
|
||
| @CLOUDMYSQL_SOURCE_DATATYPES_TEST | ||
| Scenario: Verify user should not be able to deploy and run the pipeline when plugin is configured with invalid bounding query |
There was a problem hiding this comment.
Add step to validate the log message.
|
|
||
| public class BQValidation { | ||
| public static void main(String[] args) { | ||
| // validateBQAndDBRecordValues(String schema, String sourceTable, String targetTable) |
There was a problem hiding this comment.
Remove the main method.
There was a problem hiding this comment.
I don't see any commits. How should I make sure its resolved ?
Push the changes first then when marking them as resolved add the commit link in the comment.
|
To do :
|
|
@priyabhatnagar25 @Ishakaushik-cloud Squash all the commits for final review as asked. |
| Then Close the pipeline logs | ||
| Then Validate the values of records transferred to target CloudSQLMySql table is equal to the values from source BigQuery table | ||
|
|
||
|
|
There was a problem hiding this comment.
remove extra space just one line space is enough
|
|
||
|
|
||
| public static boolean validateBQAndDBRecordValues(String sourceTable, String targetTable) | ||
| throws SQLException, ClassNotFoundException, IOException, InterruptedException, ParseException { |
There was a problem hiding this comment.
fix the indentation/spacing.
| throws SQLException, ClassNotFoundException, IOException, InterruptedException, ParseException { | ||
| getBigQueryTableData(sourceTable, bigQueryRows); | ||
| for (Object rows : bigQueryRows) { | ||
| JsonObject json = new Gson().fromJson(String.valueOf(rows), JsonObject.class); |
There was a problem hiding this comment.
Create a global Gson object and re-use it in both the methods.
| */ | ||
|
|
||
| private static void getBigQueryTableData(String table, List<Object> bigQueryRows) | ||
| throws IOException, InterruptedException { |
There was a problem hiding this comment.
fix spacing, We are using 2 space indentation.
| String dataset = PluginPropertyUtils.pluginProp("dataset"); | ||
| String selectQuery = "SELECT TO_JSON(t) FROM `" + projectId + "." + dataset + "." + table + "` AS t"; | ||
| TableResult result = BigQueryClient.getQueryResult(selectQuery); | ||
| result.iterateAll().forEach(value -> bigQueryRows.add(value.get(0).getValue())); |
There was a problem hiding this comment.
fix spacing.
| case Types.DECIMAL: | ||
| org.apache.spark.sql.types.Decimal sourceDecimal = org.apache.spark.sql.types.Decimal.fromDecimal(rsSource.getBigDecimal(currentColumnCount)); | ||
| org.apache.spark.sql.types.Decimal targetDecimal = org.apache.spark.sql.types.Decimal.fromDecimal(bigQueryData.get(jsonObjectIdx).get(columnName).getAsBigDecimal()); | ||
| Assert.assertEquals("Different values found for column : %s", sourceDecimal, targetDecimal); |
There was a problem hiding this comment.
Don't directly import it here.
| * CloudSqlMySql Plugin related step design. | ||
| */ | ||
| public class CloudMysql implements CdfHelper { | ||
| @Then("Validate the values of records transferred to target table is equal to the values from source table") |
There was a problem hiding this comment.
Why do we have 3 validation Steps here ? keep the 2 we are using and delete the unused one.
|
|
||
| String jdbcUrl = String.format( | ||
| PluginPropertyUtils.pluginProp("jdbcURL"), | ||
| database, instanceConnectionName, username, password); |
There was a problem hiding this comment.
fixing spacing in above lines.
|
|
||
| @Before(order = 2, value = "@CLOUDMYSQL_TEST_TABLE") | ||
| public static void createCloudMysqlTestTable() throws SQLException, ClassNotFoundException { | ||
| // BeforeActions.scenario.write("SQL Target table name - " + sqlTargetTableName); |
There was a problem hiding this comment.
Remove the comments
| } | ||
| System.out.println(bqSourceTable); | ||
| PluginPropertyUtils.addPluginProp("bqSourceTable", bqSourceTable); | ||
| // BeforeActions.scenario.write("BQ Source Table " + bqSourceTable + " created successfully"); |
There was a problem hiding this comment.
Remove comment
| PluginPropertyUtils.addPluginProp("bqSourceTable", bqSourceTable); | ||
| // BeforeActions.scenario.write("BQ Source Table " + bqSourceTable + " created successfully"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Remove the space.
| */ | ||
|
|
||
| public class TestSetupHooks { | ||
| public static void setTableName() { |
There was a problem hiding this comment.
If you are using a formatter then set 2 space indentation in it. We use the same indentation in all the files and here in this project 2 spaces one is used.
| @@ -35,6 +35,7 @@ public class OracleSourceSchemaReader extends CommonSchemaReader { | |||
| * Oracle type constants, from Oracle JDBC Implementation. | |||
| */ | |||
| public static final int INTERVAL_YM = -103; | |||
| public static final int TINY_BLOB = -3; | |||
There was a problem hiding this comment.
Why do we need these changes here, to handle tiny blob type in validation logic ?
| // Insert query does not return any record. | ||
| // Iterator on TableResult values in getSoleQueryResult method throws NoSuchElementException | ||
| } | ||
| System.out.println(bqSourceTable); |
There was a problem hiding this comment.
Remove this
| PluginPropertyUtils.addPluginProp("sourceTable", sourceTableName); | ||
| PluginPropertyUtils.addPluginProp("targetTable", targetTableName); | ||
| PluginPropertyUtils.addPluginProp("selectQuery", String.format("select * from %s", sourceTableName)); | ||
| System.out.println(sourceTableName); |
There was a problem hiding this comment.
Remove this as well.
| String datatypesColumns = PluginPropertyUtils.pluginProp("CloudMySqlDatatypesColumns"); | ||
| String createTargetTableQuery = "CREATE TABLE " + targetTable + " " + datatypesColumns; | ||
| statement.executeUpdate(createTargetTableQuery); | ||
| System.out.println(createTargetTableQuery); |
There was a problem hiding this comment.
Remove this.
| String datatypesColumns = PluginPropertyUtils.pluginProp("datatypesColumns"); | ||
| String createTargetTableQuery = "CREATE TABLE " + targetTable + " " + datatypesColumns; | ||
| statement.executeUpdate(createTargetTableQuery); | ||
| System.out.println(createTargetTableQuery); |
There was a problem hiding this comment.
Remove this
| String datatypesColumns = PluginPropertyUtils.pluginProp("datatypesColumns"); | ||
| String createSourceTableQuery = "CREATE TABLE " + sourceTable + " " + datatypesColumns; | ||
| statement.executeUpdate(createSourceTableQuery); | ||
| System.out.println(createSourceTableQuery); |
There was a problem hiding this comment.
Remove this.
|
@bharatgulati Do a dry run of the data validation logic and if everything is good raise an external PR after cherry picking. Changes LGTM. |
| // Insert dummy data. | ||
| statement.executeUpdate("INSERT INTO " + sourceTable + " (id, lastName)" + "VALUES (1, 'Priya')"); | ||
| statement.executeUpdate("INSERT INTO " + sourceTable + " (id, lastName)" + "VALUES (2, 'Shubhangi')"); | ||
| statement.executeUpdate("INSERT INTO " + sourceTable + " (id, lastName)" + "VALUES (3, 'Shorya')"); |
There was a problem hiding this comment.
@priyabhatnagar25 @IshaCS This PR was supposed to be raised externally are we planning to raise this ?
|
|
||
| public static void createSourceTable(String sourceTable) throws SQLException, ClassNotFoundException { | ||
| try (Connection connect = getCloudSqlConnection(); Statement statement = connect.createStatement()) { | ||
| String createSourceTableQuery = "CREATE TABLE IF NOT EXISTS " + sourceTable + "(id int, lastName varchar(255), PRIMARY KEY (id))"; |
There was a problem hiding this comment.
Same here, I know these things should be flagged in review but (@priyabhatnagar25 & @IshaCS ) you both have been working on this and didn't see this is wrong it shouldn't be hardcoded.
@Ishakaushik-cloud @AnkitCLI @priyabhatnagar25