refactor: cleanup

justinmk · justinmk · commit 555acfb79f3b · 2024-03-16T19:34:29.000+01:00
- annotate types.
- rename helpers/ =&gt; utils/
  - there are already other "utils/" directories
  - helpers is a stupid name
diff --git a/packages/integration-tests/__tests__/integration.test.ts b/packages/integration-tests/__tests__/integration.test.ts
@@ -4,13 +4,13 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as http from 'http';
 
-import { attach } from 'neovim';
+import { NeovimClient, attach } from 'neovim';
 
 describe('Node host', () => {
   const testdir = process.cwd();
-  let proc;
+  let proc: cp.ChildProcessWithoutNullStreams;
   let args;
-  let nvim;
+  let nvim: NeovimClient;
 
   beforeAll(async () => {
     const plugdir = path.resolve(__dirname);
diff --git a/packages/neovim/src/api/Buffer.test.ts b/packages/neovim/src/api/Buffer.test.ts
@@ -1,9 +1,7 @@
 /* eslint-env jest */
-import * as cp from 'node:child_process';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as which from 'which';
-import { attach } from '../attach';
-import { NeovimClient } from './client';
+import * as testUtil from '../testUtil';
 
 function wait(ms: number): Promise<void> {
   return new Promise(resolve => {
@@ -25,8 +23,7 @@ try {
 }
 
 describe('Buffer API', () => {
-  let proc;
-  let nvim: NeovimClient;
+  let nvim: ReturnType<typeof testUtil.getNvim>;
 
   // utility to allow each test to be run in its
   // own buffer
@@ -48,18 +45,12 @@ describe('Buffer API', () => {
   }
 
   beforeAll(async () => {
-    proc = cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
-      cwd: __dirname,
-    });
-
-    nvim = attach({ proc });
+    testUtil.startNvim2();
+    nvim = testUtil.getNvim();
   });
 
   afterAll(() => {
-    nvim.quit();
-    if (proc && proc.connected) {
-      proc.disconnect();
-    }
+    testUtil.stopNvim2();
   });
 
   it(
@@ -406,22 +397,15 @@ describe('Buffer API', () => {
 });
 
 describe('Buffer event updates', () => {
-  let proc;
-  let nvim;
+  let nvim: ReturnType<typeof testUtil.getNvim>;
 
   beforeAll(async () => {
-    proc = cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
-      cwd: __dirname,
-    });
-
-    nvim = attach({ proc });
+    testUtil.startNvim2();
+    nvim = testUtil.getNvim();
   });
 
   afterAll(() => {
-    nvim.quit();
-    if (proc && proc.connected) {
-      proc.disconnect();
-    }
+    testUtil.startNvim2();
   });
 
   beforeEach(async () => {
diff --git a/packages/neovim/src/api/Neovim.test.ts b/packages/neovim/src/api/Neovim.test.ts
@@ -1,10 +1,9 @@
 /* eslint-env jest */
-import * as cp from 'node:child_process';
 import * as path from 'node:path';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as which from 'which';
-import { attach } from '../attach';
 import { Neovim } from './Neovim';
+import * as testUtil from '../testUtil';
 
 try {
   which.sync('nvim');
@@ -18,22 +17,15 @@ try {
 }
 
 describe('Neovim API', () => {
-  let proc;
-  let nvim: Neovim;
+  let nvim: ReturnType<typeof testUtil.getNvim>;
 
   beforeAll(async () => {
-    proc = cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
-      cwd: __dirname,
-    });
-
-    nvim = attach({ proc });
+    testUtil.startNvim2();
+    nvim = testUtil.getNvim();
   });
 
   afterAll(() => {
-    nvim.quit();
-    if (proc && proc.connected) {
-      proc.disconnect();
-    }
+    testUtil.stopNvim2();
   });
 
   it('sets transport when initialized', () => {
diff --git a/packages/neovim/src/api/Neovim.ts b/packages/neovim/src/api/Neovim.ts
@@ -1,5 +1,5 @@
 import { BaseApi } from './Base';
-import { createChainableApi } from './helpers/createChainableApi';
+import { createChainableApi } from './utils/createChainableApi';
 import { Buffer, AsyncBuffer } from './Buffer';
 import { Tabpage, AsyncTabpage } from './Tabpage';
 import { Window, AsyncWindow } from './Window';
diff --git a/packages/neovim/src/api/Tabpage.test.ts b/packages/neovim/src/api/Tabpage.test.ts
@@ -1,8 +1,7 @@
 /* eslint-env jest */
-import * as cp from 'node:child_process';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as which from 'which';
-import { attach } from '../attach';
+import * as testUtil from '../testUtil';
 
 try {
   which.sync('nvim');
@@ -16,22 +15,15 @@ try {
 }
 
 describe('Tabpage API', () => {
-  let proc;
-  let nvim;
+  let nvim: ReturnType<typeof testUtil.getNvim>;
 
   beforeAll(async () => {
-    proc = cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
-      cwd: __dirname,
-    });
-
-    nvim = attach({ proc });
+    testUtil.startNvim2();
+    nvim = testUtil.getNvim();
   });
 
   afterAll(() => {
-    nvim.quit();
-    if (proc && proc.connected) {
-      proc.disconnect();
-    }
+    testUtil.stopNvim2();
   });
 
   beforeEach(() => {});
diff --git a/packages/neovim/src/api/Tabpage.ts b/packages/neovim/src/api/Tabpage.ts
@@ -1,6 +1,6 @@
 import { BaseApi } from './Base';
 import { ExtType, Metadata, Promisify } from './types';
-import { createChainableApi } from './helpers/createChainableApi';
+import { createChainableApi } from './utils/createChainableApi';
 import { Window, AsyncWindow } from './Window';
 
 export class Tabpage extends BaseApi {
diff --git a/packages/neovim/src/api/Window.test.ts b/packages/neovim/src/api/Window.test.ts
@@ -1,9 +1,7 @@
 /* eslint-env jest */
-import * as cp from 'node:child_process';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as which from 'which';
-import { attach } from '../attach';
-import { NeovimClient } from './client';
+import * as testUtil from '../testUtil';
 
 try {
   which.sync('nvim');
@@ -17,22 +15,15 @@ try {
 }
 
 describe('Window API', () => {
-  let proc;
-  let nvim: NeovimClient;
+  let nvim: ReturnType<typeof testUtil.getNvim>;
 
   beforeAll(async () => {
-    proc = cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
-      cwd: __dirname,
-    });
-
-    nvim = attach({ proc });
+    testUtil.startNvim2();
+    nvim = testUtil.getNvim();
   });
 
   afterAll(() => {
-    nvim.quit();
-    if (proc && proc.connected) {
-      proc.disconnect();
-    }
+    testUtil.stopNvim2();
   });
 
   beforeEach(() => {});
diff --git a/packages/neovim/src/api/Window.ts b/packages/neovim/src/api/Window.ts
@@ -1,6 +1,6 @@
 import { BaseApi } from './Base';
 import { ExtType, Metadata, Promisify } from './types';
-import { createChainableApi } from './helpers/createChainableApi';
+import { createChainableApi } from './utils/createChainableApi';
 import { Tabpage, AsyncTabpage } from './Tabpage';
 import { Buffer, AsyncBuffer } from './Buffer';
 
diff --git a/packages/neovim/src/api/utils/createChainableApi.ts b/packages/neovim/src/api/utils/createChainableApi.ts
diff --git a/packages/neovim/src/api/utils/types.ts b/packages/neovim/src/api/utils/types.ts
diff --git a/packages/neovim/src/attach/attach.test.ts b/packages/neovim/src/attach/attach.test.ts
@@ -1,10 +1,9 @@
 /* eslint-env jest */
-import * as cp from 'node:child_process';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as which from 'which';
 import { attach } from './attach';
-import { Logger, logger } from '../utils/logger';
-import { NeovimClient } from '../api/client';
+import { Logger } from '../utils/logger';
+import * as testUtil from '../testUtil';
 
 try {
   which.sync('nvim');
@@ -18,29 +17,14 @@ try {
 }
 
 describe('Nvim API', () => {
-  let proc;
-  let nvim;
+  let nvim: ReturnType<typeof testUtil.getNvim>;
   let requests;
   let notifications;
 
-  function startNvim() {
-    return cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
-      cwd: __dirname,
-    });
-  }
-
-  function stopNvim(proc_: cp.ChildProcessWithoutNullStreams | NeovimClient) {
-    if (proc_ instanceof NeovimClient) {
-      proc_.command('qa!');
-    } else if (proc_ && proc_.connected) {
-      proc_.disconnect();
-    }
-  }
-
   beforeAll(async () => {
-    proc = startNvim();
+    testUtil.startNvim2();
+    nvim = testUtil.getNvim();
 
-    nvim = attach({ proc });
     nvim.on('request', (method, args, resp) => {
       requests.push({ method, args });
       resp.send(`received ${method}(${args})`);
@@ -51,8 +35,7 @@ describe('Nvim API', () => {
   });
 
   afterAll(() => {
-    stopNvim(nvim);
-    stopNvim(proc);
+    testUtil.stopNvim2();
   });
 
   beforeEach(() => {
@@ -62,20 +45,22 @@ describe('Nvim API', () => {
 
   it('console.log is monkey-patched to logger.info #329', async () => {
     const spy = jest.spyOn(nvim.logger, 'info');
+    // eslint-disable-next-line no-console
     console.log('log message');
     expect(spy).toHaveBeenCalledWith('log message');
     // Still alive?
     expect(await nvim.eval('1+1')).toEqual(2);
   });
 
   it('console.log NOT monkey-patched if custom logger passed to attach()', async () => {
-    const proc2 = startNvim();
+    const [proc2] = testUtil.startNvim(false);
     const logged: string[] = [];
+    let logger2 = {};
     const fakeLog = (msg: any) => {
       logged.push(msg);
       return logger2;
-    }
-    const logger2 = {
+    };
+    logger2 = {
       info: fakeLog,
       warn: fakeLog,
       debug: fakeLog,
@@ -84,20 +69,21 @@ describe('Nvim API', () => {
     const nvim2 = attach({ proc: proc2, options: { logger: logger2 } });
 
     const spy = jest.spyOn(nvim2.logger, 'info');
+    // eslint-disable-next-line no-console
     console.log('message 1');
     // console.log was NOT patched.
     expect(spy).toHaveBeenCalledTimes(0);
     // Custom logger did NOT get the message.
-    expect(logged).toEqual([])
+    expect(logged).toEqual([]);
 
     // Custom logger can be called explicitly.
-    nvim2.logger.info('message 2')
-    expect(logged).toEqual(['message 2'])
+    nvim2.logger.info('message 2');
+    expect(logged).toEqual(['message 2']);
 
     // Still alive?
     expect(await nvim2.eval('1+1')).toEqual(2);
 
-    stopNvim(nvim2);
+    testUtil.stopNvim(nvim2);
   });
 
   it('can send requests and receive response', async () => {
@@ -152,6 +138,7 @@ describe('Nvim API', () => {
   });
 
   it('emits "disconnect" after quit', done => {
+    const proc = testUtil.getNvimProc();
     const disconnectMock = jest.fn();
     nvim.on('disconnect', disconnectMock);
     nvim.quit();
diff --git a/packages/neovim/src/testUtil.ts b/packages/neovim/src/testUtil.ts
@@ -0,0 +1,54 @@
+import * as cp from 'node:child_process';
+import { NeovimClient } from './api/client';
+import { attach } from './attach';
+
+let proc: cp.ChildProcessWithoutNullStreams;
+let nvim: NeovimClient;
+
+/**
+ * Gets the current Nvim client being used in the current test.
+ */
+export function getNvim(): NeovimClient {
+  return nvim;
+}
+
+export function getNvimProc(): cp.ChildProcessWithoutNullStreams {
+  return proc;
+}
+
+export function startNvim(
+  doAttach: boolean = true
+): [cp.ChildProcessWithoutNullStreams, NeovimClient | undefined] {
+  const proc = cp.spawn('nvim', ['-u', 'NONE', '--embed', '-n', '--noplugin'], {
+    cwd: __dirname,
+  });
+  if (!doAttach) {
+    return [proc, undefined];
+  }
+  const nvim = attach({ proc });
+  return [proc, nvim];
+}
+
+export function stopNvim(
+  proc_: cp.ChildProcessWithoutNullStreams | NeovimClient
+) {
+  if (proc_ instanceof NeovimClient) {
+    proc_.quit();
+  } else if (proc_ && proc_.connected) {
+    proc_.disconnect();
+  }
+}
+
+export function startNvim2(): void {
+  [proc, nvim] = startNvim();
+}
+
+// TODO: use jest beforeAll/afterAll instead of requiring the tests to do this explicitly.
+export function stopNvim2(): void {
+  stopNvim(proc);
+  stopNvim(nvim);
+}
+
+// beforeAll(async () => {
+//   [proc, nvim] = testUtil.startNvim();
+// });
diff --git a/packages/neovim/src/utils/logger.ts b/packages/neovim/src/utils/logger.ts
