Skip to content

Bigtable E2E coverage as per ITN class.#1385

Merged
sahusanket merged 1 commit intodata-integrations:developfrom
cloudsufi:BigTableE2E
Apr 15, 2024
Merged

Bigtable E2E coverage as per ITN class.#1385
sahusanket merged 1 commit intodata-integrations:developfrom
cloudsufi:BigTableE2E

Conversation

@NavdeepCS
Copy link
Copy Markdown
Contributor

No description provided.

@bharatgulati bharatgulati added the build Trigger unit test build label Mar 13, 2024
@bharatgulati bharatgulati added build Trigger unit test build and removed build Trigger unit test build labels Mar 13, 2024
@AnkitCLI AnkitCLI force-pushed the BigTableE2E branch 2 times, most recently from 0acb4ce to 7a1f024 Compare March 19, 2024 11:19
@NavdeepCS NavdeepCS marked this pull request as ready for review March 28, 2024 05:22
@AnkitCLI AnkitCLI force-pushed the BigTableE2E branch 2 times, most recently from 1a3b957 to 17c5f9d Compare March 28, 2024 15:56
Comment thread src/e2e-test/java/io/cdap/plugin/utils/BigTableClient.java Outdated
try {
Instance instance = adminClient.createInstance(createInstanceRequest);
} catch (Exception e) {
e.printStackTrace();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a logger, and use LOG.error("Error occuring while creating big table instance",e);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For reference : see class GCSValidationHelper

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

added

Comment thread src/e2e-test/java/io/cdap/plugin/utils/BigTableClient.java Outdated

List<String> families = ImmutableList.of("cf1", "cf2");
if (sourceTable != null) {
createTable(connection,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT : no need of indentation, keep in single line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

done.

sourceTable, families);
}
if (sinkTable != null) {
createTable(connection,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT : no need of indentation, keep in single line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

done

* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT : Correct indentation ( check other files )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

done.

Comment thread src/e2e-test/java/io/cdap/plugin/utils/BigTableClient.java
Comment thread src/e2e-test/java/io/cdap/plugin/common/stepsdesign/TestSetupHooks.java Outdated
Comment thread src/e2e-test/java/io/cdap/plugin/common/stepsdesign/TestSetupHooks.java Outdated
Comment thread src/e2e-test/java/io/cdap/plugin/common/stepsdesign/TestSetupHooks.java Outdated
Comment thread src/e2e-test/features/bigtable/BigTableToBigTable.feature Outdated
@BigTable @BIGTABLE_SOURCE_TEST
Feature: BigTable source - Verification of BigTable to BigTable Successful Data Transfer

@BIGTABLE_SINK_TEST
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see the logic that before every test begins with deleting instance.

But CLEAN UP should happen after a test is completed.
Please change the logic to add clean up in the AFTER bock.

A test should not have any remaining from a previous run test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

added source hook.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I did not get this.

What i mean is The SOURCE TABLE and the SINK TABLE should be deleted using AFTER block.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes they are get delete after the scenarios completes.

Feature: BigTable source - Verification of BigTable to BigTable Successful Data Transfer

@BIGTABLE_SINK_TEST
Scenario: To verify data is getting transferred from BigTable source table to BigTable sink table
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How is this test different from 3rd test i.e. "To verify data is getting transferred from BigTable source table to NON existing BigTable sink" ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the difference between 1 and 3 is in first we are validating that source table is here by using validate plugin step line but in 3 we are not validating that and we are simply going as non existing tab.

*/
public class TestSetupHooks {

private static final Logger logger = LoggerFactory.getLogger(TestSetupHooks.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT : refactor logger to LOG ( static final vars are usually written in CAPs )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

done.

Then Validate data transferred to existing target bigtable table with data of source bigtable table

@BIGTABLE_SOURCE_TEST @BIGTABLE_SINK_TEST
Scenario: To verify data is getting transferred from not existing BigTable source table to BigTable sink table
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's change the name to un-validated big table source (discussed offline)

Copy link
Copy Markdown
Contributor

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 BigTableE2E branch 2 times, most recently from 6c14831 to 3810bb8 Compare April 12, 2024 15:20
@sahusanket sahusanket merged commit 3171ef2 into data-integrations:develop Apr 15, 2024
@psainics psainics deleted the BigTableE2E branch February 13, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Trigger unit test build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants