Skip to content

Lin bytes test fixes #546

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

Merged
merged 5 commits into from
Mar 28, 2025
Merged

Lin bytes test fixes #546

merged 5 commits into from
Mar 28, 2025

Conversation

jmid
Copy link
Collaborator

@jmid jmid commented Mar 28, 2025

The Lin Bytes tests are a pain point as witnessed by #544 #541 #526 #520 #518.

This PR improves on the situation by adding a Bytes.to_seq target which is a known source of domain-unsafety from the STM.domain test, yet was missing from the Lin tests. This addition means the Lin_thread test triggers much more reliably too, so it can be turned into an appropriate negative test.

Finally, shrink time of the Lin Bytes Domain test is a genuine issue #520 (it just happened again today). While trying to ramp up the targeted bindings of the corresponding STM Bytes tests today, I was playing with adding a cmd shrinker and started seeing similar longer shrink times when I added it for

 val Bytes.blit_string : string -> int -> t -> int -> int -> unit;

One problem is shrinking order: in STM, writing the shrinker by myself I could customize it to my liking, reducing the length and index integers first, and only the string secondly. Another problem is the string shrinker reducing each char repeatedly:

Shrink.string "zzzz" print_endline;;
zz
zz
zzz
nzzz
tzzz
wzzz
yzzz
znzz
ztzz
zwzz
zyzz
zznz
zztz
zzwz
zzyz
zzzn
zzzt
zzzw
zzzy

This will take quite a bit of time to reach the 'a' goal for each entry, and repeatedly start over attempting to do so.
To improve on the situation, the PR therefore adds a quick-and-dirty char-shrinker for both strings and bytes in Lin, that simply attempts to reduce a non-'a' to 'a'. As a result, shrinking for these should now work much faster out of the box.

Overall I'm hoping this fixes #544 #541 #520 #518 and thus represents a good step forward reliability-wise.
#526 is most likely due to Cygwin (signal?) issues and so remains open.

@jmid
Copy link
Collaborator Author

jmid commented Mar 28, 2025

CI summary: all 36 workflows passed

@jmid jmid merged commit 431970f into main Mar 28, 2025
36 checks passed
@jmid jmid deleted the lin-bytes-fixes branch March 28, 2025 21:10
@jmid
Copy link
Collaborator Author

jmid commented Mar 29, 2025

CI summary for merge to main: all 37 workflows passed

@jmid jmid mentioned this pull request Mar 30, 2025
@shym
Copy link
Collaborator

shym commented Mar 31, 2025

It seems that all the scheduled runs on 5.2 failed on this test this morning.
As 5.4 will enter freeze soon, maybe now is a reasonable time to retire 5.2 testing?

@jmid
Copy link
Collaborator Author

jmid commented Mar 31, 2025

My bad, sorry! 😬 The new improved ™️ Thread mode utilizes Gc.Memprof support to trigger context switching. 5.3 restored Memprof, which means the thread test works well there and on trunk - both tested by CI on PRs.
I tested locally on 5.2 at some point and then forgot about it again. Given the above, I think the Lin Thread-mode test of Bytes should just be disabled on 5.2 and earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants