refactor: externalize singleton accessors from trace writer #694

kjin · 2018-03-17T00:47:39Z

This change adds a class that abstracts the "singleton" concept and uses it for the TraceWriter singleton. This makes singletons (including the future PluginLoader class) easier to mock.

I recommend looking at each commit separately

codecov · 2018-03-17T01:03:22Z

Codecov Report

Merging #694 into master will increase coverage by 0.09%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
+ Coverage   90.06%   90.15%   +0.09%     
==========================================
  Files          29       29              
  Lines        1419     1422       +3     
  Branches      288      287       -1     
==========================================
+ Hits         1278     1282       +4     
  Misses         57       57              
+ Partials       84       83       -1

Impacted Files	Coverage Δ
src/trace-writer.ts	`88.52% <100%> (+0.06%)`	⬆️
src/util.ts	`92.85% <100%> (+1.07%)`	⬆️
src/index.ts	`90% <100%> (ø)`	⬆️

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 9d56e84...3097495. Read the comment docs.

src/util.ts

+}
+
+/**
+ * An class that provides access to a singleton.


src/util.ts

+export class Singleton<T, Config> {
+  private singleton: T|null = null;
+
+  constructor(private implementation: ClassOf<T, Config>) {}


src/util.ts

+ * Instances of this type should only be constructed in module scope.
+ */
+export class Singleton<T, Config> {
+  private singleton: T|null = null;


test/trace.ts

-let singleton: TraceWriter|null = null;
-disableTraceWriter();
+export function setTraceWriterEnabled(enabled: boolean) {
+  traceWriter['implementation'] = enabled ? TraceWriter : TestTraceWriter;


src/util.ts

+  constructor(private implementation: Constructor<T, Config>) {}

-  create(logger: Logger, config: Config&{forceNewAgent_: boolean}): T {
+  create(logger: Logger, config: Config&{forceNewAgent_?: boolean}): T {


kjin · 2018-03-19T19:54:14Z

Node 9 CI is failing only because the way it was rebuilt (through Circle workflows) doesn't allow it to correctly detect whether it should run system tests. This should not be reflected on master

kjin added 2 commits March 16, 2018 17:13

refactor: don't pass callback to traceWriter.create

5d52154

refactor: make it a little more succinct

cc27f58

googlebot added the cla: yes label Mar 17, 2018

kjin force-pushed the t-w-s branch from 084ec9d to d77d719 Compare March 17, 2018 00:54

kjin requested review from ofrobots, jinwoo and DominicKramer March 17, 2018 01:01

kjin changed the title ~~refactor: don't pass callback to traceWriter.create~~ refactor: externalize singleton accessors from trace writer Mar 17, 2018

jinwoo approved these changes Mar 19, 2018

View reviewed changes

test: revise tests to reflect new changes

1309d89

kjin force-pushed the t-w-s branch 2 times, most recently from 0776eb1 to 79af06b Compare March 19, 2018 16:31

test: add unit tests for singleton

bf30157

kjin force-pushed the t-w-s branch from 79af06b to bf30157 Compare March 19, 2018 16:45

ofrobots reviewed Mar 19, 2018

View reviewed changes

use a symbol to privatize singleton

3097495

kjin force-pushed the t-w-s branch from e03624c to 3097495 Compare March 19, 2018 18:02

kjin merged commit 5338a93 into googleapis:master Mar 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: externalize singleton accessors from trace writer #694

refactor: externalize singleton accessors from trace writer #694

kjin commented Mar 17, 2018 •

edited

Loading

Uh oh!

codecov bot commented Mar 17, 2018 •

edited

Loading

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

kjin commented Mar 19, 2018 •

edited

Loading

Uh oh!

refactor: externalize singleton accessors from trace writer #694

refactor: externalize singleton accessors from trace writer #694

Conversation

kjin commented Mar 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

kjin commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjin commented Mar 17, 2018 •

edited

Loading

codecov bot commented Mar 17, 2018 •

edited

Loading

kjin commented Mar 19, 2018 •

edited

Loading