Skip to content

Conversation

@Sita04
Copy link
Contributor

@Sita04 Sita04 commented Jan 7, 2022

Fixes #6608

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • API's need to be enabled to test (tell us)
  • Environment Variables need to be set (ask us to set them)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • Please merge this PR for me once it is approved.

@Sita04 Sita04 requested review from a team and yoshi-approver as code owners January 7, 2022 20:18
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Jan 7, 2022
Assert.assertEquals(500, client.get(PROJECT_ID, FIREWALL_RULE_CREATE).getPriority());
stdOut.close();
System.setOut(null);
ByteArrayOutputStream stdOut = new ByteArrayOutputStream();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a more consistent way would be to execute this in a before or beforeAll method.

Copy link
Contributor Author

@Sita04 Sita04 Jan 11, 2022

Choose a reason for hiding this comment

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

beforeAll() has the mechanism to clear stdout before every function.

testPatchFirewallRule() needed this as it is called from setUp() directly (to test before GCE Enforcer renders this test invalid).

// Clear system output to not affect other tests.
// Refrain from setting out to null as it will throw NullPointer in the subsequent tests.
stdOut.close();
System.setOut(new PrintStream(new ByteArrayOutputStream()));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a safer way is to save stdout and then set to previous stdout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Averi. Will include this change.

@Sita04 Sita04 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 11, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 11, 2022
@Sita04 Sita04 merged commit 495742e into main Jan 11, 2022
@Sita04 Sita04 deleted the patch-compute-firewall-rule-deletion branch January 11, 2022 15:56
Sita04 added a commit that referenced this pull request Jan 24, 2022
* fix(compute-samples): fixed flaky firewall rule deletion

* fix(compute-samples): fixed flaky firewall rule deletion

* fix(compute-samples): fixed NullPointer caused by Sysout set to null

* docs(compute-samples): included review comments

* docs(compute-samples): included review comments

* docs(compute-samples): lint fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The build failed

4 participants