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

gcscopy e2e tests scenarios #1283

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

priyabhatnagar25
Copy link
Contributor

No description provided.

@itsankit-google itsankit-google added the build Trigger unit test build label Aug 28, 2023
@bharatgulati bharatgulati force-pushed the gcscopy_e2e_tests branch 2 times, most recently from ac57eba to 4c0e34d Compare August 31, 2023 16:34
@priyabhatnagar25 priyabhatnagar25 force-pushed the gcscopy_e2e_tests branch 3 times, most recently from 1a9f1f8 to 1347cbd Compare September 1, 2023 11:40
@sau42shri sau42shri requested a review from GnsP September 1, 2023 12:40
@bharatgulati bharatgulati force-pushed the gcscopy_e2e_tests branch 2 times, most recently from 1347cbd to 76e56a7 Compare September 4, 2023 08:41
public class GCSCopy {

@Then("Validate GCSCopy successfully copies object {string} to destination bucket")
public void validateGCSCopySuccessfullyCopiedObjectToDestinationBucket(String path) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

In this step only the existence of the object in both source and target buckets is validated. Shouldn't it also verify if the contents of the objects in both buckets are identical or not ?

Choose a reason for hiding this comment

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

Hi ,
We will add the steps to verify the contents .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, Validation step to check the contents of the objects in both buckets are identical or not is added. Please review it.

Then Open and capture logs
Then Verify the pipeline status is "Succeeded"
Then Close the pipeline logs
Then Validate GCSCopy successfully copies object "gcsCsvFile" to destination bucket
Copy link
Contributor

Choose a reason for hiding this comment

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

this step does not validate whether the contents of the object was copied successfully.

Choose a reason for hiding this comment

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

Hi ,
We will add the steps to verify the contents .

Choose a reason for hiding this comment

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

Hi ,
We have added the steps to verify the contents inside the files .

String gcsObject = PluginPropertyUtils.pluginProp(path);
boolean isPresentatsource = false;
for (Blob blob : StorageClient.listObjects(sourceGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a contains check and not an equals check ?

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 equals check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was not working with equals for the scenario when sub directories toggle button is set to true, so changed to contains check. As it has to check the subdirectories also.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in case of subdirectories, we need to check if all the files in the subdirectory (in the fixtures) are present in the source bucket.

In this context, using String.contains to check if the path of any of the objects in the bucket contains (as a substring) the path of the directory containing the test CSV files, seems to be fundamentally incorrect.

I think we need to restructure this check to work correctly in case of subdirectories, rather than trying to make this check pass with a substring (String.contains) check.

Copy link
Contributor Author

@priyabhatnagar25 priyabhatnagar25 Sep 18, 2023

Choose a reason for hiding this comment

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

  1. Hi, We have created two separate validation steps for subdirectories set to true and set to false.
  2. The first validation step when subdirectory sets to true is checking in the source bucket that the subdirectory present in the source bucket along with its file is copied to the destination bucket along with its file with equals check and validating both the source and the destination bucket.
  3. The second validation step when subdirectory set to false is checking that there is one subdirectory along with its file in the source bucket but when the subdirectory toggle button is marked as false its not copying subdirectory along with its files in the destination bucket. So , the validation is checked at the destination bucket with empty check that if the destination bucket is empty it has not copied the subdirectory along with it files to the destination bucket.
  4. We have already covered the scenario for the files (not inside any folder), when the bucket is only having the files and not subdirectory and by default overwrite and copy subdirectory toggle button is marked as false.

String targetGCSBucket = TestSetupHooks.gcsTargetBucketName;
try {
for (Blob blob : StorageClient.listObjects(targetGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a contains check and not an equals check ?

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 equals check.

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment regarding checking subdirectories in the source bucket also applies here.

String sourceGCSBucket = TestSetupHooks.gcsSourceBucketName;
String gcsObject = PluginPropertyUtils.pluginProp(subdirectory);
for (Blob blob : StorageClient.listObjects(sourceGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a contains check and not an equals check ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here it is the scenario to check that the sub-directories are not copied to the destination bucket.This is not working for equals check and the scenario is failing for it when equals check is applied. It shows the assertion error stating that subdirectory is copied from the source GCS bucket.Its working fine with contains check.

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment regarding checking subdirectories in the source bucket (in the method validateGCSCopySuccessfullyCopiedObjectToDestinationBucket) also applies here.

String sourceGCSBucket = TestSetupHooks.gcsSourceBucketName;
String gcsObject = PluginPropertyUtils.pluginProp(path);
for (Blob blob : StorageClient.listObjects(sourceGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a contains check and not an equals check ?

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 equals check.

}

@Then("Validate GCSCopy failed to copy object {string} to destination bucket")
public void validateGCSCopyFailedToCopyObjectToDestinationBucket(String path) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

this only seems to check the existence of the object in the source bucket. Shouldn't it also validate the non-existence of the object in the destination bucket ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So here this step is used to check that the pipeline is getting failed when we are selecting overwrite existing file as false. Here we already have the file it the destination bucket and we are selecting overwrite option as false and the pipeline is failing.So here we already have the file in the destination bucket.

@priyabhatnagar25 priyabhatnagar25 force-pushed the gcscopy_e2e_tests branch 3 times, most recently from b0dc520 to fef2089 Compare September 11, 2023 12:51
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
src/e2e-test/features/gcscopy/GCSCopy.feature Show resolved Hide resolved
String gcsObject = PluginPropertyUtils.pluginProp(path);
boolean isPresentatsource = false;
for (Blob blob : StorageClient.listObjects(sourceGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in case of subdirectories, we need to check if all the files in the subdirectory (in the fixtures) are present in the source bucket.

In this context, using String.contains to check if the path of any of the objects in the bucket contains (as a substring) the path of the directory containing the test CSV files, seems to be fundamentally incorrect.

I think we need to restructure this check to work correctly in case of subdirectories, rather than trying to make this check pass with a substring (String.contains) check.

String targetGCSBucket = TestSetupHooks.gcsTargetBucketName;
try {
for (Blob blob : StorageClient.listObjects(targetGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment regarding checking subdirectories in the source bucket also applies here.

String sourceGCSBucket = TestSetupHooks.gcsSourceBucketName;
String gcsObject = PluginPropertyUtils.pluginProp(subdirectory);
for (Blob blob : StorageClient.listObjects(sourceGCSBucket).iterateAll()) {
if (blob.getName().contains(gcsObject)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment regarding checking subdirectories in the source bucket (in the method validateGCSCopySuccessfullyCopiedObjectToDestinationBucket) also applies here.

@priyabhatnagar25 priyabhatnagar25 force-pushed the gcscopy_e2e_tests branch 5 times, most recently from 17d18fc to 33a1f7b Compare September 21, 2023 11:11
@bharatgulati bharatgulati added build Trigger unit test build and removed build Trigger unit test build labels Sep 21, 2023
@priyabhatnagar25 priyabhatnagar25 force-pushed the gcscopy_e2e_tests branch 2 times, most recently from d5001bc to ae8cd65 Compare October 10, 2023 06:10
@GnsP GnsP merged commit e6fe8d8 into data-integrations:develop Oct 19, 2023
13 checks passed
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.

5 participants