Skip to content

Conversation

david-luna
Copy link
Contributor

Which problem is this PR solving?

The PR adding the new test workflow passed but had some issues:

  • not all packages were tested. Like pg
  • some other missed the coverage merge task which combines all the tests results per package.

Now we are having TAV failures in PRs like #3013. Is not for the missing script but it seems nx is providing a different context than npm run --workspaces

The following command

npx nx run-many --parallel=false -t test-all-versions -p @opentelemetry/instrumentation-pg

fails giving different messages, the most common is

      -- required packages ["pg-pool@^3","[email protected]"]
      -- installing ["pg-pool@^3","[email protected]"]
      -- running test "npm run test" with pg (env: {})
      
      > @opentelemetry/[email protected] test
      > nyc --no-clean mocha 'test/**/*.test.ts'
      
      
       Exception during run: Error: Cannot find module 'pg'
      Require stack:
      - /Users/david/Documents/repos/otel/opentelemetry-js-contrib/node_modules/pg-pool/index.js
      - /Users/david/Documents/repos/otel/opentelemetry-js-contrib/packages/instrumentation-pg/test/pg-pool.test.ts
      - /Users/david/Documents/repos/otel/opentelemetry-js-contrib/node_modules/mocha/lib/nodejs/esm-utils.js
      - /Users/david/Documents/repos/otel/opentelemetry-js-contrib/node_modules/mocha/lib/mocha.js
      - /Users/david/Documents/repos/otel/op

But the following command succeds

npm run --if-present test-all-versions:with-services-env -w @opentelemetry/instrumentation-pg

Is just a guess but maybe nx is also modifying the node_modules tree and affecting installations of the packages from test-all-versions.

Short description of the changes

  • add a script to calculate affected packages
  • use the output to run the test script in series
  • add missing merge coverage script in packages that need it

Copy link

codecov bot commented Sep 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.44%. Comparing base (2300dc1) to head (3bc5538).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3017      +/-   ##
==========================================
+ Coverage   90.54%   91.44%   +0.89%     
==========================================
  Files         112      146      +34     
  Lines        6083     8192    +2109     
  Branches     1349     1846     +497     
==========================================
+ Hits         5508     7491    +1983     
- Misses        575      701     +126     
Flag Coverage Δ
instrumentation-aws-sdk 86.48% <ø> (?)
instrumentation-cucumber 92.72% <ø> (?)
instrumentation-ioredis 91.08% <ø> (?)
instrumentation-mongoose 90.74% <ø> (?)
instrumentation-pg 95.95% <ø> (?)
instrumentation-redis 85.26% <ø> (?)
instrumentation-winston 79.60% <ø> (+50.74%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

"test": "nx run-many -t test",
"test:ci:affected": "nx affected -t test",
"test:browser": "nx run-many -t test:browser",
"test:browser:ci:affected": "nx affected -t test:browser",
"test-all-versions": "nx run-many --parallel=false -t test-all-versions",
Copy link
Contributor

@Ugzuzg Ugzuzg Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nx sets process.env.npm_config_legacy_peer_deps = true, which messes with installation in tav. I had to unset it in the tav patch to make everything work. Could also affect regular tav installs.

@@ -22,8 +22,9 @@
"test-v5-v6": "nyc mocha --require '@opentelemetry/contrib-test-utils' 'test/mongoose-common.test.ts' 'test/**/mongoose-v5-v6.test.ts'",
"test-v7-v8": "nyc mocha --require '@opentelemetry/contrib-test-utils' 'test/mongoose-common.test.ts' 'test/**/mongoose-v7-v8.test.ts'",
"test:with-services-env": "cross-env NODE_OPTIONS='-r dotenv/config' DOTENV_CONFIG_PATH=../../test/test-services.env npm test",
"test-all-versions": "tav",
"test-all-versions": "tav",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
"test-all-versions": "tav",
"test-all-versions": "tav",

@trentm trentm merged commit 2fec990 into open-telemetry:main Sep 4, 2025
19 checks passed
@david-luna david-luna deleted the workfow-tav-fix branch September 4, 2025 21:41
trentm added a commit to trentm/opentelemetry-js-contrib that referenced this pull request Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants