chore(deps): remove unused cpx dependencies and update used ones to cpx2

[pichlermarc]

Which problem is this PR solving?

• replaces cpx with cpx2 where it's used, removes cpx where it's not used anymore.

Supersedes #4506

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.84%. Comparing base (e49c4c7) to head (5136509). Report is 9 commits behind head on main.

:exclamation: Current head 5136509 differs from pull request most recent head c3ba45d

Please upload reports for the commit c3ba45d to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4510      +/-   ##
==========================================
- Coverage   93.32%   92.84%   -0.49%     
==========================================
  Files          84      328     +244     
  Lines        1694     9481    +7787     
  Branches      349     2031    +1682     
==========================================
+ Hits         1581     8803    +7222     
- Misses        113      678     +565     

see 269 files with indirect coverage changes

[pichlermarc]

~~Keeping as draft as I'll have to try that out on windows.~~ Edit: started working after a branch update

[pichlermarc]

LGTM.

(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.)

Yep, opened a follow-up issue: #4533

[pichlermarc]

Hmm, interestingly it failed on windows again. :thinking: converting back to draft.

[trentm]

The windows failure was a timeout:

  RequireInTheMiddleSingleton
    register
      ✔ should return a hooked object
      core module
        AND module name matches
          ✔ should call `onRequire`
        AND module name does not match
          ✔ should not call `onRequire`
      core module with sub-path
        AND module name matches
          ✔ should call `onRequire`
      non-core module
        AND module name matches
          ✔ should call `onRequire` (2809ms)
      non-core module with sub-path
        AND module name matches
          1) should call `onRequire`

  instrumenting a scoped package
    ✔ should patch internal file and main module (122ms)
    ✔ should patch main module


  50 passing (6s)
  1 failing

  1) RequireInTheMiddleSingleton
       register
         non-core module with sub-path
           AND module name matches
             should call `onRequire`:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (D:\a\opentelemetry-js\opentelemetry-js\experimental\packages\opentelemetry-instrumentation\test\node\RequireInTheMiddleSingleton.test.ts)
      at processImmediate (node:internal/timers:476:21)

I think mocha has a default timeout of 2s. Even running these tests locally on my dev macOS laptop, those particular test cases are slow:

  RequireInTheMiddleSingleton
    register
      ✔ should return a hooked object
      core module
        AND module name matches
          ✔ should call `onRequire`
        AND module name does not match
          ✔ should not call `onRequire`
      core module with sub-path
        AND module name matches
          ✔ should call `onRequire`
      non-core module
        AND module name matches
          ✔ should call `onRequire` (1043ms)
      non-core module with sub-path
        AND module name matches
          ✔ should call `onRequire` (582ms)

Also, interestingly, the preceding it('should call `onRequire`' test has an explicit timeout of 30s set. My guess is that was set originally to cope with a slow CI runner occasionally failing. So a suggestion would be to add a similar explicit longer timeout to avoid a flaky test:

diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts
index 1ba41bd0..75cd0ad6 100644
--- a/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts
+++ b/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts
@@ -178,7 +178,7 @@ describe('RequireInTheMiddleSingleton', () => {
             modulePath,
             baseDir
           );
-        });
+        }).timeout(30000);
       });
     });
   });

[github-actions[bot]]

This PR 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.

[pichlermarc]

Also, interestingly, the preceding it('should call `onRequire`' test has an explicit timeout of 30s set. My guess is that was set originally to cope with a slow CI runner occasionally failing. So a suggestion would be to add a similar explicit longer timeout to avoid a flaky test.

yep that seems to work, marking it as ready-to-review again.