chore(instrumentation-fs): remove node 14 promises check from tests (#…

…1413)
open-telemetry · Feb 28, 2023 · 7180353 · 7180353
1 parent 40461ce
commit 7180353
Show file tree

Hide file tree

Showing 2 changed files with 121 additions and 131 deletions.
diff --git a/plugins/node/instrumentation-fs/test/fsPromises.test.ts b/plugins/node/instrumentation-fs/test/fsPromises.test.ts
@@ -28,9 +28,6 @@ import tests, { FsFunction, TestCase, TestCreator } from './definitions';
 import type { FPMember, EndHook } from '../src/types';
 import { assertSpans, makeRootSpanName } from './utils';
 
-const supportsPromises =
-  parseInt(process.versions.node.split('.')[0], 10) >= 14;
-
 const TEST_ATTRIBUTE = 'test.attr';
 const TEST_VALUE = 'test.attr.value';
 
@@ -45,109 +42,107 @@ const tracer = provider.getTracer('default');
 const memoryExporter = new InMemorySpanExporter();
 provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter));
 
-if (supportsPromises) {
-  describe('fs/promises instrumentation', () => {
-    let contextManager: AsyncHooksContextManager;
-    let fsPromises: typeof FSPromisesType;
-    let plugin: Instrumentation;
+describe('fs/promises instrumentation', () => {
+  let contextManager: AsyncHooksContextManager;
+  let fsPromises: typeof FSPromisesType;
+  let plugin: Instrumentation;
 
-    beforeEach(async () => {
-      contextManager = new AsyncHooksContextManager();
-      context.setGlobalContextManager(contextManager.enable());
-      plugin = new Instrumentation(pluginConfig);
-      plugin.setTracerProvider(provider);
-      plugin.enable();
-      fsPromises = require('fs/promises');
-      assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
-    });
+  beforeEach(async () => {
+    contextManager = new AsyncHooksContextManager();
+    context.setGlobalContextManager(contextManager.enable());
+    plugin = new Instrumentation(pluginConfig);
+    plugin.setTracerProvider(provider);
+    plugin.enable();
+    fsPromises = require('fs/promises');
+    assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+  });
 
-    afterEach(() => {
-      plugin.disable();
-      memoryExporter.reset();
-      context.disable();
-    });
+  afterEach(() => {
+    plugin.disable();
+    memoryExporter.reset();
+    context.disable();
+  });
 
-    const promiseTest: TestCreator<FPMember> = (
-      name: FPMember,
-      args,
-      { error, result, resultAsError = null, hasPromiseVersion = true },
-      spans
-    ) => {
-      if (!hasPromiseVersion) return;
-      const rootSpanName = makeRootSpanName(name);
-      it(`promises.${name} ${error ? 'error' : 'success'}`, async () => {
-        const rootSpan = tracer.startSpan(rootSpanName);
+  const promiseTest: TestCreator<FPMember> = (
+    name: FPMember,
+    args,
+    { error, result, resultAsError = null, hasPromiseVersion = true },
+    spans
+  ) => {
+    if (!hasPromiseVersion) return;
+    const rootSpanName = makeRootSpanName(name);
+    it(`promises.${name} ${error ? 'error' : 'success'}`, async () => {
+      const rootSpan = tracer.startSpan(rootSpanName);
 
-        assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
-        await context
-          .with(trace.setSpan(context.active(), rootSpan), () => {
-            // eslint-disable-next-line node/no-unsupported-features/node-builtins
-            assert(
-              typeof fsPromises[name] === 'function',
-              `Expected fsPromises.${name} to be a function`
-            );
-            return Reflect.apply(fsPromises[name], fsPromises, args);
-          })
-          .then((actualResult: any) => {
-            if (error) {
-              assert.fail(`promises.${name} did not reject`);
-            } else {
-              assert.deepEqual(actualResult, result ?? resultAsError);
-            }
-          })
-          .catch((actualError: any) => {
+      assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+      await context
+        .with(trace.setSpan(context.active(), rootSpan), () => {
+          // eslint-disable-next-line node/no-unsupported-features/node-builtins
+          assert(
+            typeof fsPromises[name] === 'function',
+            `Expected fsPromises.${name} to be a function`
+          );
+          return Reflect.apply(fsPromises[name], fsPromises, args);
+        })
+        .then((actualResult: any) => {
+          if (error) {
+            assert.fail(`promises.${name} did not reject`);
+          } else {
+            assert.deepEqual(actualResult, result ?? resultAsError);
+          }
+        })
+        .catch((actualError: any) => {
+          assert(
+            actualError instanceof Error,
+            `Expected caugth error to be instance of Error. Got ${actualError}`
+          );
+          if (error) {
             assert(
-              actualError instanceof Error,
-              `Expected caugth error to be instance of Error. Got ${actualError}`
+              error.test(actualError?.message ?? ''),
+              `Expected "${actualError?.message}" to match ${error}`
             );
-            if (error) {
-              assert(
-                error.test(actualError?.message ?? ''),
-                `Expected "${actualError?.message}" to match ${error}`
-              );
-            } else {
-              actualError.message = `Did not expect promises.${name} to reject: ${actualError.message}`;
-              assert.fail(actualError);
-            }
-          });
-        rootSpan.end();
-        assertSpans(memoryExporter.getFinishedSpans(), [
-          ...spans.map((s: any) => {
-            const spanName = s.name.replace(/%NAME/, name);
-            const attributes = {
-              ...(s.attributes ?? {}),
-            };
-            attributes[TEST_ATTRIBUTE] = TEST_VALUE;
-            return {
-              ...s,
-              name: spanName,
-              attributes,
-            };
-          }),
-          { name: rootSpanName },
-        ]);
-      });
-    };
+          } else {
+            actualError.message = `Did not expect promises.${name} to reject: ${actualError.message}`;
+            assert.fail(actualError);
+          }
+        });
+      rootSpan.end();
+      assertSpans(memoryExporter.getFinishedSpans(), [
+        ...spans.map((s: any) => {
+          const spanName = s.name.replace(/%NAME/, name);
+          const attributes = {
+            ...(s.attributes ?? {}),
+          };
+          attributes[TEST_ATTRIBUTE] = TEST_VALUE;
+          return {
+            ...s,
+            name: spanName,
+            attributes,
+          };
+        }),
+        { name: rootSpanName },
+      ]);
+    });
+  };
 
-    const selection: TestCase[] = tests.filter(
-      ([name, , , , options = {}]) =>
-        options.promise !== false && name !== ('exists' as FsFunction)
-    );
+  const selection: TestCase[] = tests.filter(
+    ([name, , , , options = {}]) =>
+      options.promise !== false && name !== ('exists' as FsFunction)
+  );
 
-    describe('Instrumentation enabled', () => {
-      selection.forEach(([name, args, result, spans]) => {
-        promiseTest(name as FPMember, args, result, spans);
-      });
+  describe('Instrumentation enabled', () => {
+    selection.forEach(([name, args, result, spans]) => {
+      promiseTest(name as FPMember, args, result, spans);
     });
+  });
 
-    describe('Instrumentation disabled', () => {
-      beforeEach(() => {
-        plugin.disable();
-      });
+  describe('Instrumentation disabled', () => {
+    beforeEach(() => {
+      plugin.disable();
+    });
 
-      selection.forEach(([name, args, result]) => {
-        promiseTest(name as FPMember, args, result, []);
-      });
+    selection.forEach(([name, args, result]) => {
+      promiseTest(name as FPMember, args, result, []);
     });
   });
-}
+});
diff --git a/plugins/node/instrumentation-fs/test/fsPromisesHooks.test.ts b/plugins/node/instrumentation-fs/test/fsPromisesHooks.test.ts
@@ -24,9 +24,6 @@ import * as sinon from 'sinon';
 import type * as FSPromisesType from 'fs/promises';
 import type { FsInstrumentationConfig } from '../src/types';
 
-const supportsPromises =
-  parseInt(process.versions.node.split('.')[0], 10) >= 14;
-
 const createHookError = new Error('createHook failed');
 const createHook = sinon.spy((_functionName: string) => {
   throw createHookError;
@@ -82,41 +79,39 @@ const assertFailingCallHooks = (expectedFunctionName: string) => {
 // We are hard-coding this because Node 14 and below does not include `constants` in `fsPromises`.
 const fsConstantsR_OK = 4;
 
-if (supportsPromises) {
-  describe('fs/promises instrumentation: hooks', () => {
-    let plugin: Instrumentation;
-    let fsPromises: typeof FSPromisesType;
-
-    beforeEach(async () => {
-      plugin = new Instrumentation(pluginConfig);
-      plugin.setTracerProvider(provider);
-      plugin.setConfig(pluginConfig as FsInstrumentationConfig);
-      plugin.enable();
-      fsPromises = require('fs/promises');
-      createHook.resetHistory();
-      endHook.resetHistory();
-      assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
-    });
+describe('fs/promises instrumentation: hooks', () => {
+  let plugin: Instrumentation;
+  let fsPromises: typeof FSPromisesType;
+
+  beforeEach(async () => {
+    plugin = new Instrumentation(pluginConfig);
+    plugin.setTracerProvider(provider);
+    plugin.setConfig(pluginConfig as FsInstrumentationConfig);
+    plugin.enable();
+    fsPromises = require('fs/promises');
+    createHook.resetHistory();
+    endHook.resetHistory();
+    assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+  });
 
-    afterEach(() => {
-      plugin.disable();
-      memoryExporter.reset();
-    });
+  afterEach(() => {
+    plugin.disable();
+    memoryExporter.reset();
+  });
 
-    it('should not fail the original successful call when hooks throw', async () => {
-      // eslint-disable-next-line node/no-unsupported-features/node-builtins
-      await fsPromises.access('./test/fixtures/readtest', fsConstantsR_OK);
+  it('should not fail the original successful call when hooks throw', async () => {
+    // eslint-disable-next-line node/no-unsupported-features/node-builtins
+    await fsPromises.access('./test/fixtures/readtest', fsConstantsR_OK);
 
-      assertSuccessfulCallHooks('access');
-    });
+    assertSuccessfulCallHooks('access');
+  });
 
-    it('should not shadow the error from original call when hooks throw', async () => {
-      // eslint-disable-next-line node/no-unsupported-features/node-builtins
-      await fsPromises
-        .access('./test/fixtures/readtest-404', fsConstantsR_OK)
-        .catch(assertNotHookError);
+  it('should not shadow the error from original call when hooks throw', async () => {
+    // eslint-disable-next-line node/no-unsupported-features/node-builtins
+    await fsPromises
+      .access('./test/fixtures/readtest-404', fsConstantsR_OK)
+      .catch(assertNotHookError);
 
-      assertFailingCallHooks('access');
-    });
+    assertFailingCallHooks('access');
   });
-}
+});