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.

Sign up for GitHub

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

Jump to bottom

build: write logs to separate file #230

Merged
merged 2 commits into from
Sep 20, 2018
Merged

build: write logs to separate file #230

merged 2 commits into from
Sep 20, 2018

Conversation

callmehiphop
Copy link
Contributor

We're seeing some unexpected errors come up in our syslog file. This PR runs tests against a different log file to prevent unexpected results.

@JustinBeckwith I'm not sure if we should continue to dig into the error we're seeing, but it seems to be related to the CI environment and not the client library itself.

Sep 20 07:44:04 verdaccio puppet-agent[659]: Could not request certificate: Failed to open TCP connection to puppet:8140

@callmehiphop callmehiphop requested review from JustinBeckwith and a team September 20, 2018 19:46
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 20, 2018
@ghost ghost assigned callmehiphop Sep 20, 2018
@JustinBeckwith
Copy link
Contributor

@callmehiphop Yeah... so 2 things there.

  1. Moving to a log specifically for testing logging makes a lot of sense. That way other things in the project can't interfere with what we're expecting.

  2. Yeah, this was me :( I had created a GCE VM with verdaccio on it in our project, and didn't realize that could potentially interfere with our e2e tests. I have deleted the VM.

Thank you for digging into this.

ofrobots
ofrobots approved these changes Sep 20, 2018
View reviewed changes
Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

LGTM once comment is addressed.

system-test/logging.js Outdated
@@ -281,7 +281,7 @@ describe('Logging', () => {
});

describe('logs', () => {
const log = logging.log('syslog');
const log = logging.log('systestlog');

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@codecov
Copy link

codecov bot commented Sep 20, 2018

Codecov Report

Merging #230 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #230   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines         369    369           
=====================================
  Hits          369    369

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff0c5ea...a3301e5. Read the comment docs.

@JustinBeckwith JustinBeckwith merged commit 677ccad into googleapis:master Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JustinBeckwith JustinBeckwith JustinBeckwith approved these changes

@ofrobots ofrobots ofrobots approved these changes

Assignees

@callmehiphop callmehiphop

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@callmehiphop @JustinBeckwith @ofrobots @googlebot