4 ms·
> it's basically solving the ,,tests not pass'' problem by changing the tests themselves. False. 0 test files were deleted. 0 pre-existing tests were skipped,
by Jarred 5mo ago
> it's basically solving the ,,tests not pass'' problem by changing the tests themselves.
False.
0 test files were deleted. 0 pre-existing tests were skipped, todo’d, or had assertions removed. 5 new tests were added in test.skip/test.todo state to track known not-yet-fixed bugs in the port that lacked test coverage before.
The merge changed 28 test files in total.
+1,312 lines
−141 lines
Most of that +1,312 is new tests.
The depth-of-recursion tests for TOML/JSONC parsers went from 25_000 -> 200_000 because Rust’s smaller stack frames (LLVM lifetime annotations let the optimizer reuse stack slots) mean 25k levels no longer reaches the 18 MB stack on Windows.
- xiphias2 5mo agoThat's great! It's too bad you haven't structured the commits and pull requests a bit differently so that it's easier to review the exact changes, but I hope it goes well. For example doing the test refactorings in a first pull request, and using something like test.xfail that is first fails then after the merge succeeds (but the test code itself doesn't change). Also I have seen some tests getting stricter, which is again not a problem, but separating to a different pull request would have improved the reviewability significantly for a runtime that many people and companies depend on. I'm sorry you were downvoted by HN and your comment got ,,dead'', that's not the way to review things.
- kenloef 5mo agoWe're keeping this honest and chill, no worries. What is "most of that "? Why did you feel the need to produce so much detail about a single category of tests?