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

fs: set fs.watchFile poll interval to 1s #45536

Closed
wants to merge 1 commit into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Nov 20, 2022

I couldn't find any reference in the libuv repo for the reason or usage of 5007ms interval. Hopefully this improves the test.parallel/test-fs-watch-recursive tests.

Hopefully, fixes #45535

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Nov 20, 2022
@mscdex
Copy link
Contributor

mscdex commented Nov 20, 2022

The comment says libev not libuv. libev is the eventing library that node used to use before libuv existed.

@anonrig
Copy link
Member Author

anonrig commented Nov 20, 2022

The comment says libev not libuv. libev is the eventing library that node used to use before libuv existed.

Thanks. I thought that was a typo! Is the deleted comment still valid?

@mscdex
Copy link
Contributor

mscdex commented Nov 20, 2022

Is the deleted comment still valid?

Well, according to the current libev HEAD branch it still uses 5.007 seconds.

I think it would be better to look at modifying the test in question to solve the problem instead of changing timing like this that could potentially and subtly break things for people.

@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 21, 2022
@anonrig
Copy link
Member Author

anonrig commented Nov 21, 2022

Thank you for the awesome reference @mscdex. There are some things, you can't learn from Google. 🎉

BTW, I didn't understand the logical reasoning for the 5s delay for polling. For my own benefit, I'd appreciate it if someone can explain it to me. On the other hand, I'm going to run this branch couple of times, to validate if the flaky test repeats in this pull request.

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate flaky test.parallel/test-fs-watch-recursive
3 participants