Add tests validating that append-only hash table collections are enumerated preserving insertion order.

[eiriktsarpalis]

One useful property that Dictionary<K,V> and HashSet<T> happen to satisfy is that append-only instances enumerate their entries following their insertion order. It means that these types behave nicely when used in the context of applications like serialization and others. In fact, keeping this invariant felt important enough that we decided to not pursue a possible performance optimization that would result in breaking it.

This PR adds testing that validates that this behavior holds.

[Copilot]

Pull Request Overview

This PR adds tests to verify that append‐only instances of Dictionary and HashSet preserve insertion order during enumeration.

• Adds a new test method in HashSet.NonGeneric.Tests.cs to validate HashSet insertion order.
• Adds a new test method in Dictionary.Tests.cs to validate Dictionary enumeration order for entries, keys, and values.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/libraries/System.Collections/tests/Generic/HashSet/HashSet.NonGeneric.Tests.cs	Introduces a parameterized test to confirm that a HashSet enumerates elements in the order they were inserted.
src/libraries/System.Collections/tests/Generic/Dictionary/Dictionary.Tests.cs	Adds a parameterized test to check that Dictionary, its Keys, and Values enumerations preserve insertion order.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

[kg]

Great to see a test for this. Do we want to verify also that removal doesn't affect order as long as no insertions happen after removal? I don't know if anyone currently relies on that behavior, but I can imagine writing an application that relies on it.

I know we have tests that verify the ability to remove without invalidating enumerators, but I don't remember seeing test coverage for 'removal doesn't perturb iteration order'. Maybe 'it doesn't invalidate enumerators' implies 'it doesn't perturb order' so we're good.

Re: the comment I left earlier, upon some more thought we want to be careful that the test is something that would fail for a naive implementation.

The naive (current, I think?) GetHashCode implementation for int is int GetHashCode() => this; which means for sequential keys, if you select buckets by doing hashcode % buckets.Length (which we do, more or less, and many naive hashtables do), then as long as you have enough buckets, you will accidentally enumerate sequentially.

The way the test is currently written, for size 10 it would have passed on the vectorized unordered dictionary, and at sizes 1000 and 1000000 it would have failed but only because we're not pre-sizing the dictionary. If the dictionary were pre-allocated with space for 1000000 items and never grew, it might've still passed even though the vectorized dictionary isn't ordered, due to the bucket selection being hashcode % buckets.Length.

I'm not sure how best to write a simple test without this defect. The simdhash and vectorized dictionary tests both generate sets of unique random numbers, which is fairly expensive to do when we're talking about a million items and is just generally not fast compared to what you're doing here.

If we drop the 1 million items scenario or lower it to say 10000 items we could use 'set of unique random numbers' and then be confident that we're testing what we want to test. But I don't want to suggest that you should do that since it's kind of a pain to write.

[eiriktsarpalis]

Do we want to verify also that removal doesn't affect order as long as no insertions happen after removal?

I don't think we want to overspecify things here. What is important from my perspective is the behavior of append-only collections since that's what's important in the context of, say, serialization.

[eiriktsarpalis]

/ba-g test failures unrelated