nodejs
test: add test for `Module._stat`
Module._stat landed in https://github.com/nodejs/node/pull/44537 without a test, so this change adds one.
Signed-off-by: Darshan Sen [email protected]
@aduh95 aren't mustCalls for making sure that a callback passed to a function gets called? This one's totally synchronous, that's why I tested this completely based on the change in return values.
we need the loader hook API to stabilize
It’s pretty close. Once https://github.com/nodejs/node/pull/44710 and https://github.com/nodejs/node/pull/43772 land and we allow some time for baking, that’s all that’s on our list before declaring the API stable: https://github.com/nodejs/loaders#status
To be clear, the PR that exposed Module._stat has already landed and it has been released to v18 and v16, I'm not sure why a test for such a thing wouldn't be accepted. Do y'all have suggestions on what needs to be changed in the test for it to be accepted? I don't think it's a good idea to have untested features laying around. If y'all are not happy that the feature exists, would y'all be acceptive of a PR that reverts https://github.com/nodejs/node/pull/44537?
(As for reverting, I strongly object - I made the original PR for a reason, it shouldn't be reverted)
Where's the suggestion for reverting?
Even if we eventually revert (not that I'm suggesting we do) it would be preferable to revert both the feature and its test together, I think, so we have the history.
@GeoffreyBooth
Where's the suggestion for reverting?
I posted the comment about reverting in https://github.com/nodejs/node/pull/44713#issuecomment-1287069974 in case we are not comfortable with exposing Module._stat.
Even if we eventually revert (not that I'm suggesting we do) it would be preferable to revert both the feature and its test together, I think, so we have the history.
Yes but for that we would need to land the test first. If you take a look at the contents of https://github.com/nodejs/node/pull/44537 you would see that it landed without any tests, which is why I thought of sending a PR to add some.