Skip to content

test: cover the parser, scanner and collections to 80% - #39

Draft
Trosper3 wants to merge 104 commits into
ReduxISU:mainfrom
Trosper3:test/coverage-80
Draft

Trosper3 wants to merge 104 commits into
ReduxISU:mainfrom
Trosper3:test/coverage-80

Conversation

@Trosper3

@Trosper3 Trosper3 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Adds tests for the untested parts of the parser, grammar and collections; stacks on #28, and #31 (rbs) merges after it with the coverage floor at 80.

Features

Issue Handling

Equal sets such as {1,2} and {2,1} used to get different hash codes,
because GetHashCode hashed ToString(), which lists elements in the
order they were written. Any set of sets could then miss a lookup:
{{1,2}}.Contains({2,1}) was False and {{1,2},{2,1}} kept both.

Sets now combine their element hashes with an order-independent sum,
lists hash their elements in order, and values hash their text.

Equals now also checks that both sides are the same kind (value, list
or set). Before, an empty set compared equal to an empty list and to
any plain value.

Adds a test project (tests/SPADE.Tests, xUnit) with tests for both.

Closes ReduxISU#9
Since d7e982e, an unorderedcross membership check rejected every
normal undirected edge such as {a,b}. The added line checked whether
each node inside the edge was a list, and plain nodes never are.

The check now looks at the edge itself: a plain value is not an
edge, so it fails the constraint instead of crashing while being
enumerated. List edges such as (a,b) are still accepted, as in the
released 0.1.2, because existing grammars for directed graphs rely
on it.

Closes ReduxISU#12
An unorderedcross member is an unordered pair, so it must be a set
such as {a,b}. A list such as (a,b) is an ordered pair, which is what
cross describes, so it is now rejected here too. This is what
d7e982e set out to do.

Grammars for directed graphs that use unorderedcross with list edges
need to switch to cross.

Refs ReduxISU#12
"N is int" checked nothing at all: int reaches the constraint as a set
descriptor, and the "is" branch only handled the set, list and int
keywords. So abc, {1} and (1,2) all passed. It now requires a single
value that parses as an integer.

"S is set" accepted a plain value, because a value is not ordered. It
now requires an unordered collection.

"is list" already rejected values and sets; it is now covered by tests.

Closes ReduxISU#10
Replaces the substring-based reader in UtilCollection(string) with
InstanceScanner, which walks the input once.

Whitespace (ReduxISU#2): spaces, tabs and line breaks around { } ( ) , and at
either end of the input are skipped, so {2, 3} now holds 3 and a
trailing new line no longer breaks an instance. Inside a value,
whitespace next to [ ] ; : = is dropped, plain spaces are kept (New
York), and a run with a tab or line break becomes one space. These
match the rules Redux applies before parsing today.

Errors (ReduxISU#4): malformed input throws SpadeParseException with the
position, what was expected and what was found, e.g. "expected , or }
at position 5, found end of input" for {1,2. Input that used to be
misread without an error, such as {1,2 or {1,2}junk, is now rejected.

Time and depth (ReduxISU#7): parsing is linear in the input's length, and
ToString builds its text in one buffer. Nesting deeper than 256 levels
is rejected instead of overflowing the stack and crashing the process.
40,000 elements took 104 seconds before; 100,000 now take well under
a second.

Closes ReduxISU#2
Closes ReduxISU#7
Refs ReduxISU#4
…tion

Input that is well formed but doesn't fit the definition string threw
whatever happened to fail: InvalidOperationException ("Cannot index
into a set"), ArgumentOutOfRangeException, or a bare Exception whose
message said PLACEHOLDER. All of these now throw SpadeParseException
with a message that names what failed:

- A format such as (N,E) checks that it got a list of exactly that
  many elements. A longer list used to be accepted and the extra
  elements dropped.
- Constraint failures quote the constraint as written, e.g.
  "Constraint E subset N unorderedcross N failed: {1,9} is not a
  member". Long values are shortened.
- subset on a plain value, and cross on anything that isn't a pair,
  fail the constraint instead of crashing.

Closes ReduxISU#4
A set drops repeated elements, so {3,3,2,2} was read as {3,2}. Problems
defined on multisets, such as Partition and Subset Sum, then solved a
different instance from the one written.

A definition string can now mark a collection with "is multiset":

    {N | N is multiset, N subset int}

reads {3,3,2,2} as four numbers. It applies before the other
constraints wherever it is written. Without it, braces still mean a
set, so nothing changes for existing definitions or callers.

To make this possible, a set read from an instance string keeps its
elements as written, and AsMultiset() turns them into a multiset.
Multisets count every occurrence in Count(), enumeration and
ToString(), compare by count in any order, and have order-independent
hash codes. Union and Intersect use the larger and smaller count. New
CountOf() and IsSubsetOf() compare by count, so {7,7} is in {7,7,1}
but not in {7,1}. A set and a multiset are never equal, and "is set"
rejects a multiset.

Adds the multiset keyword to coll.g4; the lexer is regenerated with
ANTLR 4.13.2, the version that generated the existing files.

Closes ReduxISU#1
…stance

StringParser.Definition reads the definition string on first use and returns
its outline and each named part's kind, object, path and constraints, so a
caller can build an editor for a problem before any instance exists.
…xception

SpadeParseException gets Part, Element, MissingValue and ExpectedIn, so a
caller can find the part an error belongs to without reading the message.
Failures inside a nested set belong to the outer part and member. Messages
are unchanged.

Closes ReduxISU#19
"x is int", "S subset int" and UtilCollection.parseInt used the machine's culture, so a culture with another minus sign could refuse "-5". They now share IntValue, which parses with CultureInfo.InvariantCulture as NumberValue does for number (ReduxISU#23).
rbs runs audit, format-check, lint (dotnet build) and unit tests with a
coverage floor inside the repo's devcontainer, the same way it does for
Redux. It reports without failing (soft) while ci.yml stays the gate;
publish.yml keeps doing the NuGet release, since rbs has no NuGet push.

- SPADE.slnx: the dotnet engine needs a solution. It names only
  DiscreteParser.csproj for now; the test project joins it with the first
  fix PR. ci.yml and publish.yml name DiscreteParser.csproj explicitly, so
  the MSB1011 a bare `dotnet build` now gives does not affect them.
- rbs.toml: coverage floor 70; the goal is 80%+.
- .devcontainer: SDK 10 (reads .slnx, builds the net6.0 library) plus the
  .NET 8 runtime the tests run on.
- .gitignore: rbs's .rbs/ work directory and report.md.
Whitespace only: dotnet format SPADE.slnx, so rbs's format-check starts clean. Brace placement, indentation and blank lines; no code changes.
SPADE.slnx now names tests/SPADE.Tests, so rbs's unit-test stage runs it.
coverlet.collector writes the cobertura report the coverage floor reads;
without it the project gets no coverage gate at all. Same version as
Redux's test project.
Whitespace only: dotnet format SPADE.slnx on this branch's code.
Whitespace only: dotnet format SPADE.slnx on this branch's code.
Whitespace only: dotnet format SPADE.slnx on this branch's code.
… threads

Freezing used to happen only as a side effect of hashing, so a list that
was built but never hashed could still be changed while other threads
read it. Freeze() freezes a collection and everything inside it on
purpose and returns it; IsFrozen() reports it. Documentation/threads.md
says what is safe to share (frozen collections, sets) and what isn't
(a StringParser while it parses, a collection still being built), with
the parse-once, freeze, then share pattern.
UtilCollection's union (sets and multisets), cross, the assertPair /
assertCount / assertOrdered / assertUnordered checks, NotEquals and the
set-of-string-sets constructor had no tests. They now check the result
of each operation, the error message for mixing a set with a list or a
multiset, and that list union and intersection still say they aren't
implemented rather than guessing at an order.
Definitions using union and intersection were never parsed in a test.
SetOperationTests checks that each accepts the right elements and that a
failure names the part, the element, the missing value and the side it
is missing from (ReduxISU#19), and does the same for a cross pair with one bad
half.

DefinitionDescriptionTests now covers DefinitionPart.ToString, a
definition that is a list of values, and the exact message and position
for lexer and parser errors.

GrammarListenerTests walks real definitions with a collBaseListener
subclass, pinning down the tree coll.g4 gives: rule and token order,
left-nested binary operators, logical operators with not, list and set
literals, and that malformed definitions are counted as syntax errors.
This is the generated parser's code, reached through grammars it reads.
coll.g4 accepts these, but StringParser.parse doesn't handle them:
logical operators and not throw NullReferenceException, a set or list
literal after subset throws InvalidOperationException (Stack empty),
"x in A" is never checked, and a malformed definition is printed to the
console and read as best ANTLR can instead of throwing. Each test states
the wanted behaviour and is skipped, with today's behaviour as the
reason, until it's fixed.
Line coverage, counted the way rbs counts it (generated parser
included), is now about 93%, up from about 82%. The floor was 70% with
80% as the goal; 80% is now the floor.
StringParser kept ANTLR's default error listener on the definition
string, which printed the error to the console and recovered with a best
guess, so "{N | N is set" (missing brace), "{N | N is set#}" and
"{N | N is set}}" were accepted. ReduxISU#4 rules out best-guess parsing, and
Definition already rejected the same strings.

The throwing listener moves out of DefinitionDescription into its own
internal class, with one Parse method both use. The StringParser
constructor now reads the definition string through it, so a malformed
one throws the same FormatException as Definition, naming the string and
the position of the first error, before any instance is parsed. Nothing
is printed. parse() walks the tree read at construction instead of
reading the definition string again on every call.
StringParser now rejects a malformed definition string when it is made,
and DefinitionDescriptionTests (ReduxISU#24) checks the same three strings,
their exact messages and that nothing is printed. The skipped theory
here would only repeat them, so it goes; the three gaps left are ReduxISU#40,
ReduxISU#41 and ReduxISU#14.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests

1 participant