Run kson-lib tests against Rust and Python bindings#327
Open
aochagavia wants to merge 5 commits intomainfrom
Open
Run kson-lib tests against Rust and Python bindings#327aochagavia wants to merge 5 commits intomainfrom
kson-lib tests against Rust and Python bindings#327aochagavia wants to merge 5 commits intomainfrom
Conversation
We were accidentally converting the whole exception to a string, so the final error was something along the lines of "IllegalArgumentException: <the actual error>". This commit corrects that.
Since the `kson-lib` tests are written in Kotlin, we run them against Python in an indirect way: - We LLM-generated a JSON Schema based on the `kson-lib` API (with minimal manual tweaks). - We LLM-generated a clone of `kson-lib`'s `Kson.kt` that internally sends POST requests to a server based on the JSON Schema (with minimal manual tweaks). We called it `kson-lib-http` and wrote a descriptive readme for it. - We LLM-generated a Python server that receives requests and uses the bindings to handle them (with minimal manual tweaks). - We created a `:lib-python:kson-lib-tests` project responsible for actually running the `kson-lib` tests against the Python server. This setup is a bit convoluted, and I'm not sure I'd choose it for a different project. However, next to it being useful for testing, it doubles as an experiment to see how the KSON API could look like if we were to expose it through a text-based API (currently JSON, in the future... KSON!)
Here we follow the same approach used for testing the `lib-python` bindings. There is some code duplication in `ServerExtension.kt`, but I think we can tolerate that for now. Here is what we did: - We LLM-generated a Rust server that receives requests and uses the bindings to handle them (with minimal manual tweaks). - We created a `:lib-rust:kson-lib-tests` project responsible for actually running the `kson-lib` tests against the Rust server.
The previous commits implemented an alternative KSON implementation over HTTP and a mechanism to run the `kson-lib` tests against our bindings. However, the whole setup was somewhat hacky (duplicated class definitions, reusing source code files instead of using proper abstractions, etc). This commit is meant to straighten everything up: - The `kson-lib` project has been split. KSON's public interface (with no implementation) lives in `kson-service-api` while the implementation stays at `kson-lib` (which now depends on `kson-service-api`). - The `kson-lib-http` project has been renamed to `kson-http`. It now implements the interface from `kson-service-api` and hence has significantly less lines of code. - The `kson-lib` tests have been moved wholesale to an independent `kson-service-tests` project. There, they live now as an abstract class that can be inherited by other projects (currently `kson-lib`, `lib-rust/kson-lib-tests` and `lib-python/kson-lib-tests`). When extracting `kson-service-api` from `kson-lib`, I tried to avoid any changes to the public interface. Some things had to change anyway, though: - Some types used to have internal or private constructors, but those need to be public now the types live in `kson-service-api`. Otherwise consumers of the API (e.g., `kson-lib`) would be unable to build them. The types are: `Position`, `Message`, `EmbedRule`, `Token`, `Analysis`, `KsonObject`, `KsonArray`, `KsonString`, `KsonNumber.Integer`, `KsonNumber.Decimal`, `KsonBoolean`, `KsonNull`, `KsonEmbed`. - The `EmbedRule` type used to validate the json pointer upon construction and return an error in case it was invalid. This is no longer possible, because `kson-service-api` doesn't know how to validate json pointers. This means that `KsonService.format` no longer can assume `EmbedRule`s are valid. I have modified the `Kson.format` implementation from `kson-lib` to ignore embed blocks with invalid json pointers (the function's return type is `String` and we are avoiding exceptions, so this is the most sensible behavior I could come up with under those constraints). An alternative would be to make `EmbedRule` an interface, asking each implementor to ensure instances of the interface carry valid json pointers. - I had to introduce a `SchemaValidatorService`. I think the concept is sound, but the name is ugly. Ideas welcome! PS 1: while this commit is focused on getting the tests working, it lays the foundation for alternative KSON clients (e.g., KSON over KSON). All they have to do is implement the interfaces from `kson-services-api`. PS 2: we had to update the Krossover version to fix bugs we discovered along the way.
Collaborator
Author
|
@dmarcotte I pushed an additional commit that I'd very much like your feedback on (even if it's only based on the commit msg). Btw the CI failure is related to a type error in the Python bindings. I don't have much time left this week to look into it, but I wanted to get the commit out there as soon as possible for a high-level review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See the individual commits (and their corresponding messages) for the implementation details. Note that the first one is merely a fix that I decided to pick up in this PR given it's so small.
Some remarks and questions:
kson-libtest suite / migrate tests over from the core library.I'm wondering about the most effective way to help our future selves when the KSON API grows and the tests fail to compile. People will most likely be confused when adding functionality toIntroducing proper Kotlin abstractions instead of crudely reusing source files solved this issue.Kson.ktbreaks the tests (they break when the public API oflib-ksondiverges fromlib-kson-http). Also, people will not necessarily have an easy time updating the JSON schema (provided they remember at all) and the test HTTP servers provided by the bindings. I thought about leaving instructions in one of the readmes (including prompts that could fully automate the updates), but even that doesn't cut it IMO (there will still be confusion). Ideas appreciated!