[Repo Assist] fix: enable and fix experimental tests for issues #115 and #116#240
Merged
gdziadkiewicz merged 2 commits intomasterfrom Mar 14, 2026
Conversation
SkewBinomialHeapTest.fs (closes #115): - Remove broken iComparableGen() helper that tried to generate IComparable values via FsCheck — FsCheck cannot automatically generate interface instances, causing a runtime exception in all three property tests that depended on it. - Replace with Arb.generate<int> throughout the module-level emptyOnly / nonEmptyOnly generators. - Add Gen.choose(0, 12) / Gen.length1thru12 size bounds (consistent with the project guidance in FsCheckProperties.fs) so the generators stay fast at 10,000 test cases. - Promote the three ptestPropertyWithConfig tests to testPropertyWithConfig: * 'toList returns the same as toSeq |> List.ofSeq' * 'merge throws when both heaps have diferent ordering' * 'Equality reflexivity' DListTest.fs (closes #116): - The three ofSeq ptest tests were checking internal DList node structure, not observable behaviour. The current ofSeq implementation builds a left-leaning tree (via snoc) while the expected value was a right-leaning tree (built with cons), so they would always fail when enabled. - Rewrite each test to compare DList.toSeq output against the expected element sequence [0;1;2;3;4], which correctly captures the contract of ofSeq without coupling tests to the internal representation. - Promote all three from ptest to test. Test status: - FSharpx.Collections.Tests: 706 passed, 6 skipped, 0 failed - Experimental DList: 14 passed, 0 ignored, 0 failed - Experimental SkewBinomialHeap: 29 passed, 0 ignored, 0 failed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Mar 10, 2026
gdziadkiewicz
approved these changes
Mar 14, 2026
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
1 participant
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.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Fixes two long-standing experimental test issues by making the tests actually run.
SkewBinomialHeap — closes #115
Root cause: The module-level
nonEmptyOnlygenerator callediComparableGen(), which attempted to useArb.generateto produceSystem.IComparableinterface instances. FsCheck cannot automatically generate values for interface types and throws:This caused all three
ptestPropertyWithConfigtests that depended on this generator to fail as soon as they were enabled, which is why they had been left asptest.Fix:
iComparableGen().Arb.generate<'T>/Gen.nonEmptyListOf <| iComparableGen()withArb.generate(int)in the module-levelemptyOnly/nonEmptyOnlygenerators.Gen.choose(0, 12)/Gen.length1thru12size bounds (consistent with the project guidance inFsCheckProperties.fs: "recommend a range of size 1–12 for lists used to build test data structures") to keep the generators fast enough for 10,000 FsCheck rounds.ptestPropertyWithConfig→testPropertyWithConfig:"toList returns the same as toSeq |> List.ofSeq""merge throws when both heaps have diferent ordering""Equality reflexivity"DList — closes #116
Root cause: The three
ptest "test ofSeq …"tests asserted structural equality against a right-leaningJointree built withcons, butDList.ofSeqbuilds a left-leaning tree viasnoc. BecauseDList.op_Equalitycompares node structure, not just element sequence, the tests would always fail when enabled.Fix: Rewrite each test to check observable behaviour —
DList.ofSeq input |> DList.toSeq |> Seq.toList = [0;1;2;3;4]— which is the correct contract forofSeqwithout coupling tests to internal representation. Promote all three fromptesttotest.Test Status
FSharpx.Collections.TestsThe 6 skipped core tests are pre-existing skips unrelated to this change.
Closes #115
Closes #116