Conversation
previously, the stderr would be interlaced with the test runner's output, which sucks when that's the part that actually matters. now, we capture stdout and stderr, and print it with `println`, which is nicely displayed by the test runner
mainly because this merges their dead code evaluation, so `common` no longer has "unused functions". also they are literally the same test cases. why were they separated lol
|
lmao https://github.com/joshprk/jsonkdl/actions/runs/16548564711/job/46799978335?pr=9 error[E0507]: cannot move out of a shared reference
--> tests/arbitrary_precision.rs:77:47
|
77 | literal.parse::<i128>().map_err(|err| *err.kind()),
| ^^^^^^^^^^^ move occurs because value has type `IntErrorKind`, which does not implement the `Copy` trait
|
help: consider cloning the value if the performance cost is acceptable
|
77 - literal.parse::<i128>().map_err(|err| *err.kind()),
77 + literal.parse::<i128>().map_err(|err| err.kind().clone()),test failure because that was just merged seemingly right before the current stable Rust. i will fix it, because this also applies to the compiler version that is currently in nixpkgs. |
this also undoes 2107289 because KdlValue cannot have a custom repr, oopsie, those functions are inseparable after all.
627512c to
87fcfba
Compare
|
the actual tests are expected to fail when ran. as previously stated, this is due to a |
|
Hi, sorry for the wait. I'll take a look at this soon. I notice that the CI build didn't pass and you mentioned it, should I wait a bit longer or is it ready for review? |
|
Yeah. This is ready for review. As stated, tests are failing due to a bug in kdl-rs which can't parse documents with arbitrary precision numbers (only output them), and my tests include round-trip tests to ensure they parse correctly. I looked manually at the test outputs; it seems correct. No worries about the wait; i only really care about using this for nixpkgs, which doesn't support arbitrary precision anyways, so it's not like this is blocking anything. Take your time. |
|
After reviewing this, I think the best thing to do is annotate the clearly working test cases so we can compare the output text to something we verified in order to pass CI tests (since it would be unbecoming for a parsing tool to have failing CI). It seems adding arbitrary precision to the upstream kdl-rs is non-trivial (or they can make a new KdlValue variant?), and we should instead make a tracking issue for it until they are able to address it. Let me know if you have any concerns about annotating the arbitrary precision test cases with verified outputs that we can diff with the jsonkdl output. Otherwise, I'll co-author this PR to add the annotations. Thanks for sending in this PR! |
i've made the test code a bit nicer:
target/tmpof the actual cargo workspace, instead oftarget/test_outputsin the repo root. (i have my clone of this repo in a workspace; so this was bugging me)fancyfeature ofmiette, but it's only in test code.jsonkdlitself is still minimal, depending only onserde_jsonandkdl.value_reprbeing part of theKdlEntry.#inf/#-inf/#nanlike i proposed in correctness: exotic IEEE-754 values #8value_repr, we also need to insert a leading space in the entry. this is normally implicit, but not with a customvalue_repr.value_repris clunky, as it forces me to think about document formatting kdl-org/kdl-rs#130kdl-rsfails to parse it.