[instrumentation] replace `cpx2` with a local module in `tests/fixtures/` exclusively for testing

[pichlermarc]

In @opentelemetry/instrumentation we test RequireInTheMiddleSingleton by using the cpx2 package. https://github.com/open-telemetry/opentelemetry-js/blob/63d74cdc366fc337be66c1a766a2cc8e5275a85a/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts#L42-L43

As we don't use the cpx2 package anywhere else anymore, having a local test package would help us get rid of that dependency

To complete this issue we need to:

• [ ] add a local package that mimics how cpx2 is structured for testing in ./tests/fixtures/
• [ ] update the tests to use that package instead
• [ ] remove the 'cpx2' devDependency from package.json

---

It looks like cpx2 is only used as a test package for RequireInTheMiddleSingleton, so picking some other module, or manually creating a local module in a fixtures/ subdir and using that directly would work as well. However, using cpx2 seems fine.

Originally posted by @trentm in https://github.com/open-telemetry/opentelemetry-js/pull/4510#pullrequestreview-1920997060

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[Annosha]

@pichlermarc To began working on this I want to make sure to move in right direction. "add a local package that mimics how cpx2 is structured for testing in ./tests/fixtures/" For this purpose I have to create a local module as a test fixture? A fake version of cpx2 inside ./tests/fixtures/.

Question: Do we prefer a particular JS testing framework for opentelemetry-js?

[pichlermarc]

@pichlermarc To began working on this I want to make sure to move in right direction. "add a local package that mimics how cpx2 is structured for testing in ./tests/fixtures/" For this purpose I have to create a local module as a test fixture? A fake version of cpx2 inside ./tests/fixtures/.

Yes, it looks like I made a mistake when I was writing this - it should not go into ./tests/fixtures/ but rather into .test/node/node_modules. You'll see that there are already two packages in there. You'd add a fake module there. 🙂

That can be a simple commonjs package named something like test-non-core-module that has a lib directory, with an index.js that defines some module.exports similar to how cpx2 is structured (see https://github.com/bcomnes/cpx2/blob/b487cce0b30b51a28647dfdf4757df14b70043b9/lib/index.js). You can just have one export there and have it require another file (which takes the place of copy-sync) that just defines a function as its module.exports. The actual code of the function there can even be a no-op. What we're testing here is just the hooks that should be called when require is called for a package that's being instrumented.

Then you can change the test setup and test to use the new fake test-non-core-module package instead of cpx2.

Question: Do we prefer a particular JS testing framework for opentelemetry-js?

We do use mocha and usually don't accept any changes that start using another framework - so that should stay the same. The goal of this issue is to change the existing tests, there's no need to introduce any new ones 🙂

[Annosha]

@pichlermarc Please reviewing my local changes, the code is compling without errors or warnings but, @opentelemetry/instrumentation:test failing with AssertError: expected stub to be called with exact arguments Normalized the paths for discrepancies between win and linux based paths differences. Please check my changes here for your feedback.. Test log is attached below. log-error.txt TIA!

[pichlermarc]

The test-module that was introduced on that branch does not match the way the layout is in cpx2.

In cpx2 the file structure is:

package.json
lib/
 |- index.js
 |- copy-sync.js

and in package.json there's an entry for main which points to lib/index.js. So when require.resolve() will return <full-path>/cpx2/lib which is why the path is then resovled with '..' to get the basePath of the package.

In test-non-core-module the file structure is:

index.js
lib/
 |- copy-sync.js

To mimic the same behavior you can simply move the index.js file to lib/ and adapt the require there to point towards the local directory.

Then you can also add a package.json like so

{
  "name": "test-non-core-module",
  "version": "0.0.1",
  "description": "Local test module for require-in-the-middle singleton",
  "main": "lib/index.js"
}

(the important thing here is main which points it to index.js in the lib directory, which then makes the path.resovle() in the test make sense again :slightly_smiling_face:)