Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add conductor api serialization guard #46

Merged
merged 12 commits into from Dec 11, 2023

Conversation

jost-s
Copy link
Contributor

@jost-s jost-s commented Nov 16, 2023

Recently the Conductor API broke without our knowledge because of a change in a serde patch version from 1.0.180 to 1.0.181. I've added smoke tests to the serialization of App and Admin API in the Holochain repo and want to add a similar test to this repo too.

There's no Cargo lock in this repo and it specifies serde at minimum version 1.0.123. When used in the Holochain repo, serde is effectively above 1.0.181, so that's what I want to set the minimum version to in this repo.

Let me know what you think about the JSON part of the test especially.

I've removed the nix-shell command from the pipeline config as it wasn't needed for running the tests.

flake.nix Outdated Show resolved Hide resolved
@jost-s jost-s requested a review from steveej November 22, 2023 01:48
Copy link
Member

@steveej steveej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving the CI changes 👍

Copy link
Contributor

@ThetaSinner ThetaSinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, one small question about next steps with this :)

crates/holochain_serialized_bytes/Cargo.toml Outdated Show resolved Hide resolved
@jost-s jost-s merged commit 5b7c1aa into develop Dec 11, 2023
2 checks passed
@jost-s jost-s deleted the test/add-conductor-api-serialization-guard branch December 13, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants