test: replace internal API test with public API test #30742
Conversation
The internal freelist `hasItems()` test use the internal API directly. This commit changes them to use the API as it is exposed publicly. It tests the same code paths, but does it without needing `--expose-internals`. Refs: nodejs#27588 (review)
| assert.strictEqual(flist2.hasItems(), true); | ||
|
|
||
| flist2.alloc(); | ||
| assert.strictEqual(flist2.hasItems(), false); |
There was a problem hiding this comment.
Can we keep this? The additions below also aren’t public API, so it’s not really guaranteed that they keep testing this here.
|
Actually, sorry, but scrap my review. This doesn’t really improve things beyond making another internal API “more official” by testing it. |
Would you be on board with removing If yes, cool, we can do that in a semver-major. If no, then we're basically supporting public use of this API indefinitely anyway, so I'd be inclined to test the public-facing exposure of it. (At least, that was my thinking when opening this PR!) |
|
Sure, I’m good with removing it 👍 |
The internal freelist
hasItems()test use the internal API directly.This commit changes them to use the API as it is exposed publicly. It
tests the same code paths, but does it without needing
--expose-internals.Refs: #27588 (review)
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes