opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

`backwards-compatibility/*` tests are broken and lonely

Open trentm opened this issue 9 months ago • 3 comments
trafficstars

While reviewing https://github.com/open-telemetry/opentelemetry-js/pull/5456 I noticed that there are two users of "tsconfig.base.es5.json":

experimental/backwards-compatibility/node14/tsconfig.json
2:  "extends": "../../../tsconfig.base.es5.json",

experimental/backwards-compatibility/node16/tsconfig.json
2:  "extends": "../../../tsconfig.base.es5.json",

These "backcompat-node$VER" test packages started back with https://github.com/open-telemetry/opentelemetry-js/pull/1352 They are meant to test that a simple usage of NodeSDK works with an older version of @types/node than ... either the latest or whatever version are commonly using.

They are run via: npm run test:backcompat Which was run in a backcompat.yml workflow.

Until 4mo later in https://github.com/open-telemetry/opentelemetry-js/pull/1748 this was removed with the comment:

Backwards compatibility compilation checks are now included as part of the regular build so there is no reason for them to have their own GHA

I think that "are now included" is because the top-level tsconfig.json in that change included the backwards-compatiblity packages in its "references" at the time. Then, ~2y later, those references were removed in https://github.com/open-telemetry/opentelemetry-js/pull/3432

So my guess (without playing too much) is that the backward-compatibility tests haven't been run for a couple years (since PR-3432).

They are currently broken (run npm run test:backcompat).

Presumably we should drop the current node14 and node16 compat versions (our base Node.js is v18 now) and add node18 and node20 compat versions? However, we currently use "@types/node": "18.6.5", so I'm not sure of the value. An alternative might be to have a test-all-versions (TAV) test in the sdk-node package that runs a small "does NodeSDK-basically work" test with supported versions of @types/node. Then we'd not need separate packages for this.

trentm avatar Feb 12 '25 22:02 trentm