PathMap spec #2
PathMap spec #2
Conversation
chris-martin
force-pushed
the
chris/path-map
branch
from
f06e901
to
e9c23d1
Compare
| it "case 3" $ | ||
| property $ | ||
| forAll (arbitrary `suchThat` (/= "a")) $ \segment -> | ||
| let result = | ||
| PathMap.lookup | ||
| (renderPath $ Path [segment]) | ||
| (PathMap.fromList [("/{}", False), ("/a", True)]) | ||
| in counterexample ("result = " <> show result) (isNothing result) |
There was a problem hiding this comment.
The funky case as described in #1 (comment) - the spec written here describes what the code presently does, not what it should do.
| @@ -17,17 +19,21 @@ import Data.OpenApi (OpenApi, PathItem) | |||
| import Data.OpenApi qualified as OpenApi | |||
| import System.FilePath.Posix qualified as Posix | |||
|
|
|||
| newtype PathMap = PathMap | |||
| { unwrap :: Map TemplatedPath PathItem | |||
| newtype PathMap a = PathMap | |||
There was a problem hiding this comment.
Is this polymorphism just to make the tests easier to write?
FWIW, I find the property-based testing hard to follow and not very good at documenting the behaviors you uncovered. I think things would be clearer if you gave yourself a helper to create concrete PathItems and test against fixed scenarios with them.
There was a problem hiding this comment.
Is this polymorphism just to make the tests easier to write?
Yeah, it means you don't actually have to construct PathItems, since that's not relevant to what's being tested, which is only how the keys match. But we could give the polymorphic type a different name and alias type PathMap = Something PathItem to avoid complicating the user API.
I find the property-based testing hard to follow
Yeah, I just started down that road because I was suspecting that I needed quickcheck to generate some larger maps for me to find the problem cases. If wonky Ords are a problem I don't necessarily expect to be able to find it with small examples.
There was a problem hiding this comment.
If wonky
Ordsare a problem I don't necessarily expect to be able to find it with small examples
Well, you gave three cases here. I feel like one it for each of those would be sufficient.
There was a problem hiding this comment.
means you don't actually have to construct
PathItems
Well, you have to define something you can distinguish. I don't really see mempty & summary ?~ "foo" that much more difficult than some other non-PathItem type (that can be distinguished). And for test that needn't distinguish (like an isNothing assertion), then mempty is about as simple as it gets.
chris-martin
force-pushed
the
chris/path-map
branch
from
e9c23d1
to
001b36a
Compare
(In progress)