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

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

Closed
BioPaulK opened this issue Nov 20, 2024 · 4 comments · Fixed by #4950
Closed

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

BioPaulK opened this issue Nov 20, 2024 · 4 comments · Fixed by #4950
Assignees
@juan-fernandez
Labels
ci-app

Comments

@BioPaulK
Copy link

BioPaulK commented Nov 20, 2024 •
edited
Loading

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 juan-fernandez self-assigned this Nov 22, 2024
@juan-fernandez
Copy link
Collaborator

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

@juan-fernandez
Copy link
Collaborator

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
Copy link
Author

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

@juan-fernandez juan-fernandez mentioned this issue Nov 27, 2024
Merged
1 task
@juan-fernandez
Copy link
Collaborator

thanks for the reproducible case! This made it very easy to test the fix ❤ . The fix is in #4950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@juan-fernandez juan-fernandez

Labels
ci-app
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[test optimization] Fix logic to bypass jest's require cache DataDog/dd-trace-js
2 participants
@juan-fernandez @BioPaulK