chore(samples): Major update according review comments. by sborisenkox · Pull Request #369 · googleapis/java-retail

sborisenkox · 2022-03-18T16:41:08Z

Changes in all classes:

projectId variable declaration;
unit tests logic;
minor fixes.

Fixes #367

generated-files-bot · 2022-03-18T17:32:41Z

Warning: This pull request is touching the following templated files:

.kokoro/presubmit/samples.cfg

chenlei1216 · 2022-03-19T01:17:18Z

+    String bucketName = System.getenv("BUCKET_NAME");
+    String gcsBucket = String.format("gs://%s", System.getenv("BUCKET_NAME"));
+    String gcsErrorBucket = String.format("%s/errors", gcsBucket);
+    String defaultCatalog = String.format(


rename to branchName

chenlei1216 · 2022-03-19T01:21:29Z

+    // TODO(developer): Replace these variables before running the sample.
+    String projectId = ServiceOptions.getDefaultProjectId();
+    String bucketName = System.getenv("BUCKET_NAME");
+    String defaultCatalog =


Rename to branchName

chenlei1216 · 2022-03-19T01:27:03Z

  public static void main(String[] args) throws IOException, InterruptedException {
+    // TODO(developer): Replace these variables before running the sample.
+    String projectId = ServiceOptions.getDefaultProjectId();
+    String defaultCatalog =


chenlei1216 · 2022-03-19T01:33:46Z

      }
    } catch (BigQueryException e) {
-      System.out.printf("Dataset '%s' was not deleted.%n%s", datasetName, e);
+      System.out.printf("%nDataset '%s' was not deleted.%n%s", datasetName, e);


nit: is this intended?

chenlei1216 · 2022-03-23T01:52:33Z

    // TO CHECK ERROR HANDLING USE THE JSON WITH INVALID USER EVENT: gcsEventsObject =
    // "user_events_some_invalid.json"

+    CreateTestResources.main();


I'm not sure if this would work though:

It will be expensive and slow to recreate everything for each single test.

If a parallel RemoveTestReourcesTest is running in between this line and the next line, there's still a chance that the tests will fail.

So unless we can prevent RemoveTestReourcesTest from being run, we got a chance for test failures. I'm thinking if it's OK for us to just simply ignore/remove CreateTestReourcesTest and RemoveTestReourcesTest as it may affect other tests, and this is just for an one-time set up only, not being used in future code samples.

As an option, we can remove these 2 tests.
If it acceptable for coverage percentage, let's do it.

@Neenu1995 The create and remove samples are one-time utility scripts to recreate the test assets. I agree that we don't need to run these every time, and that's not the intent of these - we want to have persistent assets in our repo.

Can these tests be removed / or annotated to ignore running? Also, will this cause any issues with code coverage?

If you plan to have persistent assets, then I believe the CreateTestReourcesTest and RemoveTestReourcesTest is never used again. In that case, it is better to remove the code entirely.

But if you see a use case where this code may be used again, it is fine to keep it with an ignored annotation.

Thanks all! Given that CreateTestReourcesTest and RemoveTestReourcesTest is removed, this line can be removed now.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

chenlei1216 · 2022-03-28T19:33:30Z

    String tableId = "events";
    // TO CHECK ERROR HANDLING USE THE TABLE OF INVALID USER EVENTS: tableId = "events_some_invalid"

+    CreateTestResources.main();


Can probably remove it now

chenlei1216 · 2022-03-28T19:34:43Z

    // TO CHECK ERROR HANDLING USE THE JSON WITH INVALID USER EVENT: gcsEventsObject =
    // "user_events_some_invalid.json"

+    CreateTestResources.main();


Thanks all! Given that CreateTestReourcesTest and RemoveTestReourcesTest is removed, this line can be removed now.

chenlei1216 · 2022-03-28T19:34:58Z

    // TRY THE FULL RECONCILIATION MODE HERE:
    ReconciliationMode reconciliationMode = ReconciliationMode.INCREMENTAL;
+
+    CreateTestResources.main();


Can probably remove it now.

chenlei1216 · 2022-03-28T19:35:03Z

+    // TO CHECK ERROR HANDLING USE THE JSON WITH INVALID PRODUCT
+    // GCS_PRODUCTS_OBJECT = "products_some_invalid.json"
+
+    CreateTestResources.main();


Can probably remove it now.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…tail into test-changes

gcf-merge-on-green · 2022-03-29T22:16:13Z

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

sborisenkox added 5 commits March 18, 2022 18:30

Events crud changes.

dcfff96

Create/remove resources changes.

5311c8d

Search changes.

6d28d44

Import user events changes.

80eaa82

Product changes.

b2c9863

sborisenkox requested review from a team March 18, 2022 16:41

product-auto-label Bot added api: retail Issues related to the googleapis/java-retail API. samples Issues that are directly related to samples. labels Mar 18, 2022

blunderbuss-gcf Bot assigned prash-mi Mar 18, 2022

please revert this commit before merging

38da8db

Neenu1995 requested a review from a team March 18, 2022 17:32

Neenu1995 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 18, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 18, 2022

Neenu1995 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:ignore instruct owl-bot to ignore a PR labels Mar 18, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 18, 2022

chenlei1216 reviewed Mar 19, 2022

View reviewed changes

Product changes according to review comments.

a9f1412

kweinmeister added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 21, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 21, 2022

sborisenkox added 2 commits March 21, 2022 18:45

Fix createTable NPE.

ffeea12

Resources logic, minor changes.

b2a435a

Neenu1995 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 21, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 21, 2022

chenlei1216 reviewed Mar 23, 2022

View reviewed changes

Remove init resources tests.

c776f51

amanda-tarafa added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 23, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 23, 2022

Merge branch 'googleapis:main' into test-changes

d512bb6

Neenu1995 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Mar 28, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 28, 2022

yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 28, 2022

🦉 Updates from OwlBot post-processor

28b7290

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Neenu1995 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 28, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 28, 2022

chenlei1216 approved these changes Mar 28, 2022

View reviewed changes

Minor code changes.

c3b8baa

sborisenkox requested a review from kweinmeister March 29, 2022 13:20

kweinmeister added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 29, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 29, 2022

kweinmeister added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Mar 29, 2022

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 29, 2022

🦉 Updates from OwlBot post-processor

fcc22c7

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2022

gcf-owl-bot Bot added 2 commits March 29, 2022 15:49

🦉 Updates from OwlBot post-processor

d616164

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'test-changes' of https://github.com/sborisenkox/java-re…

03c8326

…tail into test-changes

Neenu1995 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2022

Neenu1995 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 29, 2022

kweinmeister approved these changes Mar 29, 2022

View reviewed changes

kweinmeister added the automerge Merge the pull request once unit tests and other checks pass. label Mar 29, 2022

gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label Mar 29, 2022

kweinmeister merged commit a0202bf into googleapis:main Mar 29, 2022

Conversation

sborisenkox commented Mar 18, 2022 • edited by kweinmeister Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

generated-files-bot Bot commented Mar 18, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gcf-merge-on-green Bot commented Mar 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sborisenkox commented Mar 18, 2022 •

edited by kweinmeister

Loading