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.

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

Windows tests are failing #3628

Closed
pichlermarc opened this issue Feb 22, 2023 · 3 comments · Fixed by #3642
Closed

Windows tests are failing #3628

pichlermarc opened this issue Feb 22, 2023 · 3 comments · Fixed by #3642
Assignees
Labels
bug Something isn't working internal priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization

Comments

@pichlermarc
Copy link
Member

Windows tests seem to be failing in CI and locally. The tests for @opentelemetry/instrumentation-http seem to make the JavaScript heap run out of memory.

How to reproduce

Using node 16.19.1 and npm 8.19.3

  • clone repo
  • npm install
  • npm run compile
  • npm run test

Failures in CI

@pichlermarc pichlermarc added bug Something isn't working internal triage labels Feb 22, 2023
@pichlermarc pichlermarc self-assigned this Feb 22, 2023
@dyladan dyladan added the priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization label Feb 22, 2023
@pichlermarc
Copy link
Member Author

pichlermarc commented Feb 23, 2023

Looks like this may be related to nyc, leaving it out drastically reduces memory usage to a few megabytes. 🤔
Looking into possible solutions other than increasing heap size, maybe we can reduce memory usage by adding some config options.

@pichlermarc
Copy link
Member Author

During local testing, I discovered that it looks like source map updates in babel are allocating a lot of memory, this is only an issue when running that with nyc. I'm not entirely sure why this only happens when nyc is involved though. 🤔

The only tests which failed in this manner are the ones from @opentelemetry/instrumentation-http, I have not seen any indicators that anything we do in @opentelemetry/instrumentation-http causes this kind of excessive memory use.

I went ahead and opened a PR to remove nyc from the test script used in the Windows test workflow, and introduced test:coverage as an additional script that runs the tests with nyc. This should fix the problem outlined in this issue.

Should we have more problems, it may be worth updating tooling (babel) and looking into the structure of the @opentelemetry/instrumentation-http tests.

@MSNev
Copy link
Contributor

MSNev commented Feb 27, 2023

I've also got a local repro of this (even with the above changes), where it will always fail on my windows machine (from a CLEAN build only), once I have a good build it continues to work.

Repo steps

git clean -xdf
npm install
npm run compile
npm run test

I (should) have a PR available shortly to fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working internal priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization
Projects
None yet
3 participants