Improve conformance testing of non-basic generators#46
Open
Liam-DeVoe wants to merge 24 commits intomainfrom
Open
Improve conformance testing of non-basic generators#46Liam-DeVoe wants to merge 24 commits intomainfrom
Liam-DeVoe wants to merge 24 commits intomainfrom
Conversation
All conformance binaries now read a `mode` field from JSON params. When mode is "non_basic", generators are wrapped with a trivial filter to lose their schema, forcing the compositional fallback path.
Report the raw vector of integers instead of computing size/min_element/max_element in the binary. nlohmann/json natively serializes std::vector<int> as a JSON array.
Only test_lists and test_hashmaps need dual-path (basic/non_basic) testing. Scalar generators do not benefit from it, so revert them to simple hegel::draw(gen).
Liam-DeVoe
changed the title
The compositional fallback paths of `vectors` and `dictionaries` did
not honor uniqueness when element/key generators lacked a schema. This
was exposed by the new dual-path conformance tests:
- `vectors({.unique = true})` could return a non-unique vector when the
element generator had been transformed (e.g. via `.filter()`).
- `dictionaries` could loop indefinitely if the key generator kept
returning the same value (for instance while Hypothesis was exploring
its zero/minimum inputs).
Both fallbacks now cap attempts and reject the test case via
`hegel::internal::assume` when the generator cannot satisfy the
requested size with unique values. `test_lists.cpp` now also threads
the `unique` parameter through to the `vectors` generator, so basic-
mode list conformance actually exercises the unique path.
DRMacIver
reviewed
Apr 15, 2026
| potentially non-unique vector, and `dictionaries` could loop for an | ||
| unbounded number of attempts when the key generator repeatedly returned | ||
| duplicates. Both now cap attempts and reject the test case via | ||
| `hegel::internal::assume` when the generator cannot satisfy the |
Member
There was a problem hiding this comment.
Claude loves doing this. It's the wrong solution. The correct solution is to actually check whether reject returned an error and raise it appropriately.
Member
There was a problem hiding this comment.
Ah, it's not using server side collections at all, no wonder.
DRMacIver
reviewed
Apr 15, 2026
| std::vector<T> result; | ||
| result.reserve(len); | ||
|
|
||
| if constexpr (requires(T a, T b) { a == b; }) { |
Member
There was a problem hiding this comment.
Things I don't love about this:
- It always uses the O(n^2) version. Mostly fine but I don't like it and don't want people relying on Hypothesis collections always being small because a) I'd like an option to change that and b) We definitely don't want that true in Antithesis.
- It silently fails to apply uniqueness if == isn't available when it should raise an error.
|
|
||
| size_t max_attempts = len * 10 + 10; | ||
| for (size_t attempts = 0; | ||
| result.size() < len && attempts < max_attempts; ++attempts) { |
Member
There was a problem hiding this comment.
Shouldn't this be using server side collections?
| potentially non-unique vector, and `dictionaries` could loop for an | ||
| unbounded number of attempts when the key generator repeatedly returned | ||
| duplicates. Both now cap attempts and reject the test case via | ||
| `hegel::internal::assume` when the generator cannot satisfy the |
Member
There was a problem hiding this comment.
Ah, it's not using server side collections at all, no wonder.
Resolves conflicts introduced by the large refactor on main (public API
rename, namespace alias convention, HegelSettings/hegel::draw) against
the dual-path conformance changes on this branch.
- RELEASE.md: combine main's minor-release description with this
branch's patch fix notes under RELEASE_TYPE: minor.
- tests/conformance/cpp/test_lists.cpp: adopt main's `gs::` alias and
hegel::draw / test_cases settings form while preserving the `mode`
dual-path wiring, the `unique` parameter, and the simplified
`{"elements": vec}` metric output (matches hegel-core's list.py.
- tests/conformance/cpp/test_hashmaps.cpp: adopt alias and the
new draw/settings form while preserving -based make_non_basic
wrapping for both keys and values.
EOF
)
Resolve conflicts between the dual-path conformance testing changes and main's refactor that passes a TestCase& to test callbacks and generator do_draw calls. For collections.h, port the unique-element retry loops in vectors() and dictionaries() to the new API (const TestCase& tc, do_draw(tc), tc.assume(...) instead of hegel::internal::assume). For test_lists.cpp, combine the dual-path (basic vs non_basic) branch selection and raw-element metrics from HEAD with the TestCase& callback signature from main.
v0.2.0 was released on main, which removed RELEASE.md. Keep only the still-unreleased `vectors`/`dictionaries` fallback uniqueness fix note.
… classes Main's big-refactor moved vectors/maps logic into VectorsGenerator and MapsGenerator classes and renamed dictionaries -> maps (with DictionariesParams -> MapsParams). This merge: - Reapplies the fallback-path uniqueness enforcement for vectors() and duplicate-key avoidance for maps() into the new do_draw() methods. - Updates test_hashmaps.cpp to call gs::maps() (was gs::dictionaries()) while preserving the mode=non_basic wrapping for keys and values.
Merged origin/main. The newly added test_settings flaky-reporting tests and the exception-message output test referenced the pre-0.3.0 API (`hegel::hegel`, `hegel::settings::HealthCheck`, `hegel::settings::Database`). Updated them to the current names: `hegel::test`, `hegel::HealthCheck`, `hegel::Database`. Combined the pending RELEASE.md entries from both sides.
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
2 participants
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.
Claude-written description
Part of a cross-repo effort to add dual-path conformance testing. Every conformance binary now reads a
modefield from its JSON params and acts on it:make_non_basichelper tometrics.h— wraps a generator in a trivial.filter()to strip its schema, forcing the compositional fallback path.get_modehelper tometrics.h— extracts the mode string from parsed JSON args.mode == "non_basic".test_booleans.cppnow parses JSON params from argv (previously took no arguments).No library source files changed — only conformance test binaries.
Note: Conformance CI may fail until the corresponding hegel-core PR merges (which adds the
modefield to conformance test params).