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

Test suite consistently failing on Node.js 12 and 14 #58

Closed
MylesBorins opened this issue Jul 21, 2020 · 5 comments · Fixed by #76
Closed

Test suite consistently failing on Node.js 12 and 14 #58

MylesBorins opened this issue Jul 21, 2020 · 5 comments · Fixed by #76
Projects

Comments

@MylesBorins
Copy link

This fails with a variety of abort traps. It would appear that the source of the issue in the old version of chokidar

@phated
Copy link
Member

phated commented Jul 21, 2020

Very weird to be introducing major breaking changes in an LTS version of the platform.

@MylesBorins
Copy link
Author

I'm tracking down exactly what is going on... but afaict the change is breaking the unmaintained version of chokidar v1 and fsevents v1.

MylesBorins added a commit to MylesBorins/glob-watcher that referenced this issue Jul 21, 2020
The current version of chokidar used by this project is broken on
Node.js v12.x and v14.x. Updating this version fixes the problem.

Fixes: gulpjs#58
Fixes: gulpjs#55
Fixes: gulpjs#49
@MylesBorins
Copy link
Author

So looking at it right now it seems like the change that broke the test suite landed in 12.18.2, which only had two changes... both related to fixing pretty bad memory leaks, and neither of which appear on surface to be Semver-Major

@MylesBorins
Copy link
Author

nodejs/node#34077

@MylesBorins
Copy link
Author

So it would appear that this is the change that introduced the breakages on 12.x

nodejs/node@6f6bf01

FWIW there are still unreliable segmentation faults on v14.x with glob-watcher before this change landed. While it is creating reliable failures with glob-watcher on 12.x, those failures only exist when using an old, unsupported, version of a dependency (chokidar). While I can see how this can be seen as a "Major Breaking Change" I really don't think it is right to categorize it that way as the dependencies you are relying on explicitly do not support more modern versions of node.

It would appear, although it is not totally obvious yet, that there are existing underlying bugs we are dealing with here that are being unearthed by these changes. Since this change is effectively fixing a rather serious memory leak on 12.x, I doubt that we are going to be looking at reverting it.

The change that landed on 14.x, that also causes similar breakages, is similarly a non-breaking change... but rather a change to internals.

@phated phated added this to In progress in v5 Jun 27, 2022
v5 automation moved this from In progress to Done May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v5
  
Done
2 participants