DataDog / dd-trace-js

JavaScript APM Tracer
https://docs.datadoghq.com/tracing/
Other
655 stars 309 forks source link

Issues with test visibility, jest, and locally-imported modules #4915

Open BioPaulK opened 6 days ago

BioPaulK commented 6 days ago

In our app, we utilize the library office-addin-mock during our test process, to mock our interactions with office addin code.

If we follow its dependency tree through office-addin-mock -> office-addin-usage-data -> applicationinsights -> diagnostic-channel-publishers, one can find that in diagnostic-channel-publishers, we import our own, local version of winston called winston.pub.

Because of this contains logic, this causes the import of ./winston.pub (and any other similarly-named local modules) to utilize shimmer._requireCoreModule instead of requireModuleOrMock from jest. The shimmer method does not work with this local import, so you end up with an error message like the following when attempting to import office-addin-mock:

    Cannot find module './winston.pub'
    Require stack:
    - /Users/paul.kirby/repos/biorender/node_modules/jest-runtime/build/index.js
    - /Users/paul.kirby/repos/biorender/node_modules/@jest/core/build/cli/index.js
    - /Users/paul.kirby/repos/biorender/node_modules/@jest/core/build/index.js
    - /Users/paul.kirby/repos/biorender/node_modules/jest/build/index.js
    - /Users/paul.kirby/repos/biorender/node_modules/@nx/jest/src/executors/jest/jest.impl.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/config/schema-utils.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/command-line/run/executor-utils.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/devkit-internals.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/assert-workspace-validity.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/project-graph/build-project-graph.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/project-graph/project-graph.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/project-graph/file-utils.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/package-manager.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/package-json.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/print-help.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/src/command-line/run/run.js
    - /Users/paul.kirby/repos/biorender/node_modules/nx/bin/run-executor.js

    Require stack:
      /Users/paul.kirby/repos/biorender/node_modules/jest-runtime/build/index.js
      /Users/paul.kirby/repos/biorender/node_modules/@jest/core/build/cli/index.js
      /Users/paul.kirby/repos/biorender/node_modules/@jest/core/build/index.js
      /Users/paul.kirby/repos/biorender/node_modules/jest/build/index.js
      /Users/paul.kirby/repos/biorender/node_modules/@nx/jest/src/executors/jest/jest.impl.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/config/schema-utils.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/command-line/run/executor-utils.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/devkit-internals.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/assert-workspace-validity.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/project-graph/build-project-graph.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/project-graph/project-graph.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/project-graph/file-utils.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/package-manager.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/package-json.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/utils/print-help.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/src/command-line/run/run.js
      /Users/paul.kirby/repos/biorender/node_modules/nx/bin/run-executor.js

    > 1 | import { OfficeMockObject } from 'office-addin-mock';
        | ^
      2 | import { type Dispatch } from 'react';
      3 | import { REQUEST_ACCESS_STATUS, type IFeatureFlag } from '@biorender/toolkit/contracts/contracts-common';
      4 | import { EVENTS } from '@biorender/analytics/contracts-analytics';

      at Function.Module._resolveFilename.sharedData.moduleResolveFilenameHook.installedValue [as _resolveFilename] (../../../node_modules/@cspotcode/source-map-support/source-map-support.js:811:30)
      at Module.Hook.Module.require (../../../node_modules/dd-trace/packages/dd-trace/src/ritm.js:64:27)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:873:19)
      at Object.<anonymous> (../../../node_modules/diagnostic-channel-publishers/dist/src/index.js:25:15)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/applicationinsights/AutoCollection/diagnostic-channel/initialization.ts:15:54)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/applicationinsights/AutoCollection/CorrelationContextManager.ts:4:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/applicationinsights/applicationinsights.ts:1:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/office-addin-usage-data/src/usageData.ts:4:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/office-addin-usage-data/src/main.ts:6:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/office-addin-mock/node_modules/office-addin-manifest/src/addInTypes.ts:4:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/office-addin-mock/node_modules/office-addin-manifest/src/main.ts:4:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/office-addin-mock/src/officeMockObject.ts:1:1)
      at Runtime.requireModuleOrMock (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (../../../node_modules/office-addin-mock/src/main.ts:4:1)
      at Runtime.require (../../../node_modules/dd-trace/packages/datadog-instrumentations/src/jest.js:875:32)
      at Object.<anonymous> (src/lib/office-addin/office-addin.test.tsx:1:1)

My educated guess is that this logic in packages/datadog-instrumentations/src/jest.js wasn't really meant to be a contains but an equals:

LIBRARIES_BYPASSING_JEST_REQUIRE_ENGINE.some(library => moduleName.includes(library))

I originally thought this was resolved in 5.26 but that turned out to be operator error.

juan-fernandez commented 4 days ago

hey @BioPaulK ! Thanks for the report. I'll have a look and get back to you as soon as possible 😄

juan-fernandez commented 1 day ago

thanks for the investigation @BioPaulK. This is indeed a bug in dd-trace-js. I'll push a fix as soon as possible. To confirm that what I do actually fixes your issue, could you provide a small reproducible case?

BioPaulK commented 14 hours ago

Here you go - npm test should cause the error. https://github.com/BioPaulK/test-ddtrace-bug