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

Increase Logging Testing stability #903

Merged
merged 2 commits into from
Nov 1, 2017
Merged

Increase Logging Testing stability #903

merged 2 commits into from
Nov 1, 2017

Conversation

kurtisvg
Copy link
Contributor

@kurtisvg kurtisvg commented Nov 1, 2017

Increase the stability of the Logging tests.

@lesv - FYI

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 1, 2017
@kurtisvg kurtisvg requested a review from lesv November 1, 2017 21:15
@kurtisvg
Copy link
Contributor Author

kurtisvg commented Nov 1, 2017

Also satisfies #885 (KMS tests should already have been addressed in #902)

Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

A few Q's, but LGTM if none of the Q's require changes.

System.out.println(logEntry);
}
entries = entries.getNextPage();
} while(entries != null);
Copy link
Contributor

Choose a reason for hiding this comment

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

There are forms of getting entries that aren't fully paged, but are infinite. I'm presuming you've verified that this is actually paged?

@@ -56,37 +59,36 @@ public void setUp() {

@After
public void tearDown() {
// Clean up created logs
deleteLog(QUICKSTART_LOG);
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to do this @before as well? incase something broke last time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm concerned that part of the reasons the tests are failing periodically is that the deletes are actually being registered after the test line is logged. By moving the cleanup to the tearDown, it should always clean up unless the build is somehow forced to stop between the file log and that this exact moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just concerned that some crash happens and it doesn't execute - so I ask Q's. If you are happy, I'm happy. (ie you don't think that will happen)

Copy link
Contributor

Choose a reason for hiding this comment

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

sgtm

@@ -39,6 +39,9 @@
@SuppressWarnings("checkstyle:abbreviationaswordinname")
public class LoggingIT {

private final String QUICKSTART_LOG = "my-log";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these go to $TEMP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the name of the log in StackDriver that it can later be referred by, not the location it's logging too.

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM

@kurtisvg kurtisvg merged commit ab7da15 into master Nov 1, 2017
@kurtisvg kurtisvg deleted the logging branch November 1, 2017 23:30
minherz pushed a commit that referenced this pull request Nov 9, 2022
…plugin to v3 (#903)

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [org.apache.maven.plugins:maven-deploy-plugin](https://maven.apache.org/plugins/) ([source](https://togithub.com/apache/maven-deploy-plugin)) | `2.8.2` -> `3.0.0` | [![age](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/compatibility-slim/2.8.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/confidence-slim/2.8.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-errorreporting).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMTcuNCIsInVwZGF0ZWRJblZlciI6IjMyLjExNy40In0=-->
minherz pushed a commit that referenced this pull request Nov 10, 2022
…plugin to v3 (#903)

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [org.apache.maven.plugins:maven-deploy-plugin](https://maven.apache.org/plugins/) ([source](https://togithub.com/apache/maven-deploy-plugin)) | `2.8.2` -> `3.0.0` | [![age](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/compatibility-slim/2.8.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/confidence-slim/2.8.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-errorreporting).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMTcuNCIsInVwZGF0ZWRJblZlciI6IjMyLjExNy40In0=-->
minherz pushed a commit that referenced this pull request Nov 10, 2022
…plugin to v3 (#903)

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [org.apache.maven.plugins:maven-deploy-plugin](https://maven.apache.org/plugins/) ([source](https://togithub.com/apache/maven-deploy-plugin)) | `2.8.2` -> `3.0.0` | [![age](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/compatibility-slim/2.8.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/org.apache.maven.plugins:maven-deploy-plugin/3.0.0/confidence-slim/2.8.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-errorreporting).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMTcuNCIsInVwZGF0ZWRJblZlciI6IjMyLjExNy40In0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants