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

wrangler e2e tests #666

Merged

Conversation

AnkitCLI
Copy link
Contributor

Wrangler e2e tests for excel ,xmltojson and json parsers.

@Vipinofficial11 Vipinofficial11 added the build Triggers unit test build label Sep 11, 2023
@AnkitCLI AnkitCLI force-pushed the Wrangler-parsing-2 branch 8 times, most recently from fc300d3 to 747a1f0 Compare September 15, 2023 09:17
@AnkitCLI AnkitCLI force-pushed the Wrangler-parsing-2 branch 2 times, most recently from c11f146 to a8a2cdf Compare October 9, 2023 08:04
@Before(order = 1, value = "@BQ_SOURCE_XML_TEST")
public static void createTempSourceBQTableXml() throws IOException, InterruptedException {
createSourceBQTableWithQueries(PluginPropertyUtils.pluginProp("CreateBQDataQueryFileXml"),
PluginPropertyUtils.pluginProp("InsertBQDataQueryFileXml"));
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@AnkitCLI AnkitCLI force-pushed the Wrangler-parsing-2 branch 5 times, most recently from 0edd9ff to 5a20db2 Compare November 29, 2023 07:45
@AnkitCLI AnkitCLI force-pushed the Wrangler-parsing-2 branch 3 times, most recently from 8222c1a to dcd8cb2 Compare December 8, 2023 15:38
@AnkitCLI AnkitCLI force-pushed the Wrangler-parsing-2 branch 3 times, most recently from ae13467 to ee7c885 Compare January 10, 2024 16:54
@itsankit-google itsankit-google added build Triggers unit test build and removed build Triggers unit test build labels Jan 11, 2024
pom.xml Outdated
@@ -83,7 +83,7 @@
<aws.sdk.version>1.11.133</aws.sdk.version>
<bigquery.connector.hadoop2.version>0.10.2-hadoop2</bigquery.connector.hadoop2.version>
<bouncycastle.version>1.56</bouncycastle.version>
<cdap.version>6.10.0-SNAPSHOT</cdap.version>
<cdap.version>6.11.0-SNAPSHOT</cdap.version>
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Member

Choose a reason for hiding this comment

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

Please revert if not required.

Copy link
Member

Choose a reason for hiding this comment

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

Actually we can change it to 6.10.0 instead since it is released now.

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 6.10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

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

Please use range of version in plugin versions and cdap data pipeline artifact version in the pipeline jsons otherwise we will need to always keep modifying the pipeline.

@itsankit-google
Copy link
Member

Please use range of version in plugin versions and cdap data pipeline artifact version in the pipeline jsons otherwise we will need to always keep modifying the pipeline.

It does not looks like this comment is addressed, the plugin & cdap-data-pipeline versions are still hardcoded.

@@ -3,7 +3,7 @@
"description": "Data Pipeline Application",
"artifact": {
"name": "cdap-data-pipeline",
"version": "6.10.0-SNAPSHOT",
"version": "[4.0.0, 5.0.0]",
Copy link
Member

@itsankit-google itsankit-google Jan 11, 2024

Choose a reason for hiding this comment

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

It should be [6.0.0, 7.0.0).

The comment applies to the whole PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -24,8 +24,6 @@ Feature: datatype parsers
Then Replace input plugin property: "project" with value: "projectId"
Then Replace input plugin property: "dataset" with value: "dataset"
Then Replace input plugin property: "table" with value: "bqSourceTable"
Then Click on the Get Schema button
Copy link
Member

Choose a reason for hiding this comment

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

why is this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the pipelines are exported.In almost all the PR's the issue is when we try to get schema then randomly in most of the fields it is going null when we click on get schema and validate button. And it also changes the schema . So just to solve that issue we removed that part from here.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by this?

when we try to get schema then randomly in most of the fields it is going null when we click on get schema and validate button. And it also changes the schema

Can you please elaborate with the help of screenshots?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the schema and validation step.

@@ -3,7 +3,7 @@
"description": "Data Pipeline Application",
"artifact": {
"name": "cdap-data-pipeline",
"version": "6.10.0-SNAPSHOT",
"version": "[6.0.0, 7.0.0]",
Copy link
Member

Choose a reason for hiding this comment

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

nit: [6.0.0, 7.0.0)

this comment applies to whole PR.

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 in all the json pipelines.

Copy link
Member

Choose a reason for hiding this comment

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

This comment is not addressed still, the upper bound should not be included [6.0.0, 7.0.0)

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.

@AnkitCLI AnkitCLI force-pushed the Wrangler-parsing-2 branch 3 times, most recently from 4e22f8e to ccd261f Compare January 23, 2024 11:45
@itsankit-google itsankit-google merged commit b1da2c2 into data-integrations:develop Jan 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Triggers unit test build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants