Repository navigation
Test suite, step 2a: more unit tests; fix double wrapping after a GC - #913
Merged
Merged
Conversation
balat
marked this pull request as ready for review
September 27, 2026 14:08
A block moved by a minor collection gets a second index in the table of the traversal. When the table is rehashed, the two entries are merged, keeping the largest index, whose substitution was still empty: a marked value shared inside the wrapped value, visited once before the collection and once after, was wrapped again and lost its sharing. The data computed for the dropped index is now kept for the other one.
It parses the names of state cookies built by make_full_cookie_name; it is exported, in the internal part of the interface, so that the two can be tested together.
Cover the JSON serialisation of cookie sets sent to client processes, the hashing of cookie values, session identifiers, the names of state cookies (built by make_full_cookie_name and parsed back by get_state_cookies for every kind of state, hierarchy, security and cookie level), and scope hierarchies created twice.
Cover Wrap: replacement of marked values, sharing, cycles, wrappers whose result is wrapped again, large values, and minor collections during the traversal, including a shared value moved between its two visits. Also cover encode_eliom_data and string_escape, whose output is embedded in single-quoted strings of page scripts: no quote, markup or control character remains in clear, and the JavaScript literal gives the original bytes back.
Cover the printing of functional and DOM nodes and their identifiers, named and global elements, custom data, escaping of text and attributes, and the escaping, in the pages for wrong parameters, of the names and values of parameters sent by the client.
Cover the options of the <eliom> element of the server configuration file (garbage collection frequencies, limits of sessions and coservices, secure cookies, subnet masks, client program options, global data caching, ignored parameters, omitted persistent storage) and the errors on wrong values and unknown options.
balat
force-pushed
the
test-unit-more
branch
from
September 27, 2026 14:25
90284a0 to
b86f13a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second step of the test suite: unit tests of pure server functions (no site, no request, no server). They found a bug in
Wrap, fixed here.Bug fix: double wrapping when the GC moves a shared value
Wrap.wraptraverses the value with a hash table indexed by block addresses. A block moved by a minor collection gets a second index; when the table is rehashed, the two entries are merged, keeping the largest index, whose substitution was still empty. A marked value shared inside the wrapped value, visited once before a collection and once after, was therefore wrapped again: the wrapper function ran twice (e.g. the data ofEliom_lazyvalues, which compute URLs, was computed twice) and the sharing was lost on the client.The fix keeps, for the retained index, what was computed for the dropped one (
on_mergecallbacks of the table). The new testwrap / minor collectionsfails on every run without it.Tests
test_cookies: JSON serialisation of cookie sets sent to client processes; hashing of cookie values (with a known SHA-256 vector); session identifiers; names of state cookies built bymake_full_cookie_nameand parsed back byget_state_cookies(now exported in the internal part ofcommon.server.mli) for every kind of state, hierarchy, security and cookie level; scope hierarchies created twice.test_wrap: replacement of marked values, sharing, cycles, nested wrapping, large values, minor collections during the traversal (with the second visit of a shared value on both the resizing and the rehashing paths of the table), a wrapper that raises;encode_eliom_data;string_escape, whose output is embedded in single-quoted strings of page scripts: no quote, markup or control character remains in clear, and the JavaScript literal gives the original bytes back, without legacy octal escapes.test_content: functional and DOM nodes and their identifiers, named and global elements, custom data, escaping, and escaping of client-sent parameter names and values in the error pages for wrong parameters.test_config: the options of the<eliom>element of the configuration file and their errors; each limit option sets its own limits and no other.46 unit tests in total. Each file was checked against mutations of the code it covers.
Next steps
add_cache_headersendsCache-Control: max-age: Ninstead ofmax-age=N; it will be fixed there with a test.