kowainik / tomland

🏝 Bidirectional TOML serialization
https://kowainik.github.io/posts/2019-01-14-tomland
Mozilla Public License 2.0
121 stars 39 forks source link

Property-based tests generate invalid TOML keys occasionally #374

Closed roberth closed 2 years ago

roberth commented 3 years ago

I've found this test failure in CI. Perhaps it hasn't been seen before, because of the random nature of property tests.

Log excerpt ``` Failures: test/Test/Toml/Codec/Combinator/Common.hs:39:5: 1) Codec: unit and property tests for bidirectional codecs, Combinator spec, Combinator.Map: Roundtrip tests, Map Text Int (tableMap): decode . encode ≡ id ✗ failed at test/Test/Toml/Codec/Combinator/Common.hs:39:5 after 38 tests and 10 shrinks. ┏━━ test/Test/Toml/Codec/Combinator/Common.hs ━━━ 28 ┃ codecRoundtripWith 29 ┃ :: forall a 30 ┃ . (Eq a, Show a) 31 ┃ => (TomlCodec a -> Text -> Either [TomlDecodeError] a) 32 ┃ -> String 33 ┃ -> (Key -> TomlCodec a) 34 ┃ -> Gen a 35 ┃ -> SpecWith (Arg Expectation) 36 ┃ codecRoundtripWith dcode typeName mkCodec genA = it label $ hedgehog $ do 37 ┃ a <- forAll genA ┃ │ fromList [ ( "\"\\\"" , 0 ) ] 38 ┃ let codec = mkCodec "a" 39 ┃ tripping a (encode codec) (dcode codec) ┃ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ┃ │ ━━━ Intermediate ━━━ ┃ │ "\n[a]\n" ┃ │ ━━━ - Original) (+ Roundtrip ━━━ ┃ │ - Right (fromList [ ( "\"\\\"" , 0 ) ]) ┃ │ + Right (fromList []) 40 ┃ where 41 ┃ label :: String 42 ┃ label = typeName ++ ": decode . encode ≡ id" This failure can be reproduced by running: > recheck (Size 37) (Seed 10155217494328621451 7554747479521254871) ```

Full log: https://hercules-ci.com/accounts/github/hercules-ci/derivations/%2Fnix%2Fstore%2F7l00v2ra2yfh0rxxnfn6aar9k0m7qmxp-tomland-1.3.2.0.drv/log?via-job=64f51676-eac8-45b1-ad55-340f00e7272c

roberth commented 3 years ago

The second attempt did not find the failure. The first attempt is on the same page. It was built on an aarch64-linux machine, but I don't think that should matter.

chshersh commented 3 years ago

@roberth Yes, this is a known test failure. Sometimes propery-based tests failed because the generated TOML key is invalid. In the error message, the key is \, which is an invalid TOML key. To fix this, we need to patch Key generator even further.

https://github.com/kowainik/tomland/blob/2b4bcc465b79873a61bccfc7131d423a9a0aec1d/test/Test/Toml/Gen.hs#L108-L131

TotallyNotChase commented 2 years ago

Is there a reason the bare generator makes sure the first character in the key is an alphabetical character? Doesn't the toml spec allow alphanumeric + dash in any position? https://toml.io/en/v0.5.0#keys