| > 1. Malignant output from react/link, https://github.com/red/red/issues/3713
> One-line fix, sure. Where's the test? You haven't really looked at the fix, nor do you understand what the issue is about...yet you think you are warranted to emit a quality judgement about it. The issue is about `react/link` function generating a too long output when it encounters an error. That output includes references to potentially heavy graphic objects spreading over dozens or hundreds of lines, while the user only needs the beginning of that output (one or two lines), in order to identify which reaction code is concerned by that error. The fix restricts printing everything using `probe code` to printing only the first 80 characters `print mold/part/flat code 80` (adding a flattening too using `/flat` refinement to remove eventual line breaks in that output). This is typically a case where no test is needed nor desired, because: * it is just about shortening a part of the output in an error report to a decent size instead of letting it go unchecked. It has no possible side-effect on anything else in the codebase. * the replaced code cannot fail in any way, as `mold/part/flat` will deliver a properly cropped and sanitized string regardless of what `code` refers too. And that is guaranteed by the unit tests on `mold` in the tests suite. [1] If someone would submit a test for such fix as a PR, it would be rejected at once, as it would be a waste of time/bandwidth/storage/resources. Its author would be viewed as very naive, or extremely dogmatic, as you are here in your posts. > 2. new-line flags are not cleared on subsequent calls, https://github.com/red/red/issues/3707
> Severl line-fix. Where's the test? On the principle, that one should have a regression test added, I agree. But I did not add one because: * this function is just for visually formatting blocks by adding a new-line marker at desired place(s). This is rarely used function, and with the `/skip` refinement, it is an extremely rare occurence in user code. In the Red codebase, out of 157 occurrences, `/skip` is present only 5 times, and only in Rebol parts, not Red parts, so unaffected by this fix. * this function has no impact on the rest of a user app code, as it is used only for pretty-printing some outputs in the console. New-line markers could, in theory, be misused to be the base of some funky algorithms, but I have never heard of anyone ever doing that in Rebol or Red world. * this simple case can be left for a contributor to softly start learning about Red language and our testing framework, while doing a useful contribution. Each missing `test.written` tag in a fixed ticket is a good opportunity for contribution. Therefore, the regression risk associated with not writting any test for this feature is extremely low. So, in such case, I am willing to trade the time to write the tests for that, for making another fix, or advancing on another feature. In a future batch-processing of those missing tests (as explained in detail in another post), or once we reach the "catch-up" stage in 0.9, if nobody did contribute it, we will write the tests for this one, in order to reach full coverage. I don't expect a dogmatic programmer to understand this kind of trade-off, anyway. > 3. stack overflow on limited append/insert, https://github.com/red/red/issues/3705
> The fix contains a single test even though the issue provided three different failure modes Correct. Though, `append` is a shortcut for `head insert tail`, so the `append` features are implemented inside `insert` native function. The provided test seems to only cover `insert/dup` case: --assert 5000 = length? head insert/dup #{} #{20} 5000
but actually, it covers both, because the implicit `tail` in `append` has no effect on an empty series like here (the `#{}` binary series). So `head insert/dup #{}` or `append/dup #{}` are both going through the same code paths internally. The only diverging path is on the last 2 changed lines of the fix. Though, other existing tests in the test suite are already covering that [2], so no worries.For the third case, it's indeed missing, and the probable explanation is that the developer missed it, as it only appears in a later comment in the ticket, and not in the main description on top of the ticket. > I remember seeing a "status.tested" which added easily a hundred lines of new code with zero tests. The only parts I can think of where hundreds of lines of new code would not be covered by at least some tests are in the View module (our GUI library). Those kind of tests are usually very costly to write, complicated to test properly (because of different rendering on different OS), and hard to automate on our headless CI servers. Though, even there, we try to do something about it, by providing a prototype "null" backend [3] for testing at least the platform-independent parts of the View engine, and we got also recently a contribution for testing some image-related features on Windows platform. [4] [1] https://github.com/red/red/tree/master/tests/source/units [2] https://github.com/red/red/blob/master/tests/source/units/se... [3] https://github.com/red/red/tree/master/modules/view/backends... [4] https://github.com/red/red/tree/master/tests/source/view |