Skip to content

Commit 78bf71d

Browse files
Smaug123claude
andcommitted
Add characterization tests for known arg-parser bugs
These tests pin the current (buggy) observable behaviour of the generated parser, each with a comment stating the desired behaviour. The planned rewrite will flip them deliberately, so every semantic change is visible in review rather than happening silently. Covers: scan abort on malformed space-separated values; duplicate flags swallowing the following token; raw FormatException escaping from list, positional, post-separator, and env-var-default conversions; environment lookups running after the parse has failed; case-sensitivity asymmetry between --help and ordinary argument matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 5c5d40c commit 78bf71d

2 files changed

Lines changed: 156 additions & 0 deletions

File tree

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
namespace WoofWare.Myriad.Plugins.Test
2+
3+
open System
4+
open NUnit.Framework
5+
open FsUnitTyped
6+
open ConsumePlugin
7+
8+
/// These tests pin down current behaviour of the generated arg parser which we believe to be
9+
/// buggy. Each test asserts what the parser *does today*, with a comment stating the desired
10+
/// behaviour. They exist so that the planned rewrite of the arg parser flips them deliberately,
11+
/// making every observable semantic change reviewable, rather than changing behaviour silently.
12+
[<TestFixture>]
13+
module TestArgParserKnownBugs =
14+
15+
let noEnv (_ : string) : string option = None
16+
17+
// DESIRED: a malformed value in `--key value` form should record the conversion error and
18+
// continue scanning, exactly as the `--key=value` form does. Instead, the scan stops dead:
19+
// `--bar` and `--baz` below are never processed, so they are spuriously reported missing.
20+
[<Test>]
21+
let ``BUG: a malformed space-separated value aborts the remainder of the scan`` () =
22+
let exc =
23+
Assert.Throws<exn> (fun () ->
24+
BasicNoPositionals.parse' noEnv [ "--foo" ; "bad" ; "--bar=present" ; "--baz=true" ]
25+
|> ignore<BasicNoPositionals>
26+
)
27+
28+
exc.Message
29+
|> shouldEqual
30+
"""Errors during parse!
31+
The input string 'bad' was not in a correct format.
32+
Required argument '--foo' received no value
33+
Required argument '--bar' received no value
34+
Required argument '--baz' received no value"""
35+
36+
// Contrast case: the same malformed value in `--key=value` form *does* continue the scan.
37+
// This test is here to document the asymmetry with the test above; this behaviour (continue
38+
// scanning after a conversion error) is the desired one.
39+
[<Test>]
40+
let ``A malformed equals-form value does not abort the scan`` () =
41+
let exc =
42+
Assert.Throws<exn> (fun () ->
43+
BasicNoPositionals.parse' noEnv [ "--foo=bad" ; "--bar=present" ; "--baz=true" ]
44+
|> ignore<BasicNoPositionals>
45+
)
46+
47+
exc.Message
48+
|> shouldEqual
49+
"""Errors during parse!
50+
The input string 'bad' was not in a correct format. (at arg --foo=bad)
51+
Required argument '--foo' received no value"""
52+
53+
// DESIRED: a duplicated flag must not consume the next token as its "value". Here the
54+
// duplicate `--baz` swallows `--foo=3`, so on top of the duplicate-arg error we get a
55+
// spurious "'--foo' received no value" error, and `--foo=3` is never parsed.
56+
[<Test>]
57+
let ``BUG: a duplicated flag consumes the following option as its value`` () =
58+
let exc =
59+
Assert.Throws<exn> (fun () ->
60+
BasicNoPositionals.parse' noEnv [ "--baz=true" ; "--baz" ; "--foo=3" ; "--bar=present" ]
61+
|> ignore<BasicNoPositionals>
62+
)
63+
64+
exc.Message
65+
|> shouldEqual
66+
"""Errors during parse!
67+
Argument '--baz' was supplied multiple times: True and --foo=3
68+
Required argument '--foo' received no value"""
69+
70+
// DESIRED: a conversion failure in a list-typed (repeatable) argument should be recorded as a
71+
// parse error like scalar conversion failures are, not escape as a raw FormatException with
72+
// no indication of which argument was at fault.
73+
[<Test>]
74+
let ``BUG: a malformed list-element value throws a raw FormatException`` () =
75+
Assert.Throws<FormatException> (fun () ->
76+
BasicNoPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "--rest" ; "notanint" ]
77+
|> ignore<BasicNoPositionals>
78+
)
79+
|> fun exc ->
80+
exc.Message
81+
|> shouldEqual "The input string 'notanint' was not in a correct format."
82+
83+
// DESIRED: as above, but for a positional argument.
84+
[<Test>]
85+
let ``BUG: a malformed positional value throws a raw FormatException`` () =
86+
Assert.Throws<FormatException> (fun () ->
87+
BasicWithIntPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "notanint" ]
88+
|> ignore<BasicWithIntPositionals>
89+
)
90+
|> fun exc ->
91+
exc.Message
92+
|> shouldEqual "The input string 'notanint' was not in a correct format."
93+
94+
// DESIRED: as above, but for a positional argument appearing after the `--` separator.
95+
[<Test>]
96+
let ``BUG: a malformed positional value after the separator throws a raw FormatException`` () =
97+
Assert.Throws<FormatException> (fun () ->
98+
BasicWithIntPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "--" ; "notanint" ]
99+
|> ignore<BasicWithIntPositionals>
100+
)
101+
|> fun exc ->
102+
exc.Message
103+
|> shouldEqual "The input string 'notanint' was not in a correct format."
104+
105+
// DESIRED: a malformed environment-variable default should be reported through the parser's
106+
// error channel (naming the environment variable), not escape as a raw FormatException.
107+
[<Test>]
108+
let ``BUG: a malformed environment-variable default throws a raw FormatException`` () =
109+
Assert.Throws<FormatException> (fun () ->
110+
ContainsBoolEnvVar.parse' (fun _ -> Some "notabool") []
111+
|> ignore<ContainsBoolEnvVar>
112+
)
113+
|> fun exc ->
114+
exc.Message
115+
|> shouldEqual "String 'notabool' was not recognized as a valid Boolean."
116+
117+
// DESIRED: once the parse is known to have failed, no further effects should run: the
118+
// environment should not be consulted for defaults. Today the env lookup runs anyway, so a
119+
// throwing `getEnvironmentVariable` masks the real parse error.
120+
[<Test>]
121+
let ``BUG: environment lookups run even after the parse has already failed`` () =
122+
let exc =
123+
Assert.Throws<exn> (fun () ->
124+
ContainsBoolEnvVar.parse' (fun _ -> failwith "env var was consulted") [ "--bool-var=notabool" ]
125+
|> ignore<ContainsBoolEnvVar>
126+
)
127+
128+
exc.Message |> shouldEqual "env var was consulted"
129+
130+
// DESIRED: help detection should use the same case-insensitive comparison as ordinary
131+
// argument matching. Today `--FOO=1` matches the field `Foo`, but `--HELP` is not help: it
132+
// falls through to ordinary (failed) key processing.
133+
[<Test>]
134+
let ``BUG: ordinary args match case-insensitively but help is case-sensitive`` () =
135+
// Case-insensitive ordinary match: this parses fine.
136+
BasicNoPositionals.parse' noEnv [ "--FOO=1" ; "--bar=x" ; "--baz=true" ]
137+
|> shouldEqual
138+
{
139+
Foo = 1
140+
Bar = "x"
141+
Baz = true
142+
Rest = []
143+
}
144+
145+
// ...but --HELP does not produce the help text.
146+
let exc =
147+
Assert.Throws<exn> (fun () -> BasicNoPositionals.parse' noEnv [ "--HELP" ] |> ignore<BasicNoPositionals>)
148+
149+
exc.Message
150+
|> shouldEqual
151+
"""Errors during parse!
152+
Trailing argument --HELP had no value. Use a double-dash to separate positional args from key-value args.
153+
Required argument '--foo' received no value
154+
Required argument '--bar' received no value
155+
Required argument '--baz' received no value"""

WoofWare.Myriad.Plugins.Test/WoofWare.Myriad.Plugins.Test.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
<Compile Include="TestCataGenerator\TestMyList2.fs" />
4545
<Compile Include="TestArgParser\TestArgParser.fs" />
4646
<Compile Include="TestArgParser\TestArgParserNegation.fs" />
47+
<Compile Include="TestArgParser\TestArgParserKnownBugs.fs" />
4748
<Compile Include="TestSwagger\TestSwaggerParse.fs" />
4849
<Compile Include="TestSwagger\TestSuccessResponse.fs" />
4950
<Compile Include="TestSwagger\TestSwaggerTypeRender.fs" />

0 commit comments

Comments
 (0)