Skip to content

Conversation

huitseeker
Copy link
Contributor

@huitseeker huitseeker commented Sep 5, 2025

This PR against #2071 implements a serde_test macro that can generate round-trip serialization tests pegged on an implementation of proptest::prelude::Arbitrary. This macro:

=> no more boilerplate tests! (and future tests are easier to define)

Note

Tests failing here are signal for #2071 (unless I messed up something in my Arbitrary impls).

TODO (after merging this PR)

  • make sure the import of the miden-serde-test-macros crate is sparse and follows best practices re: features. (I've been fast & loose),
  • check CI / Makefile to ensure those feature-gated tests are run (i.e. the failure in the present PR's CI is still observed after tweaking features as per the above),
  • have a better implementation of Arbitrary for LibraryExports (where the last field isn't always None), add one in missing places, e.g. KernelLibrary,
  • fix failing tests (see feat: implement custom sections in package format, prototype new serialization format #2071 review)

@huitseeker huitseeker requested a review from bitwalker September 5, 2025 06:13
@huitseeker huitseeker added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Sep 5, 2025
@huitseeker huitseeker force-pushed the huitseeker/add-serde-tests branch 3 times, most recently from d2e2654 to 9ad23a4 Compare September 5, 2025 08:02
@bitwalker
Copy link
Collaborator

Note that our use of serde requires a self-describing format to work properly, and bincode is not. This was one of the main motivations behind the miden-bite design.

I'd suggest we either pick a binary format this is self-describing for these tests, or punt on merging this until miden-bite (or whatever format we land on there) is merged.

@bitwalker bitwalker force-pushed the bitwalker/custom-sections branch 2 times, most recently from 1f4fea1 to aedea70 Compare September 9, 2025 21:36
… serialization tests pegged on an implementation of proptest::prelude::Arbitrary. This macro:

- supports generic arguments passed as attributes,
- uses bincode as the test serializer (I expect this will be replaced by #2130)

=> no more boilerplate tests! (and future tests are easier to define)
@bitwalker bitwalker force-pushed the huitseeker/add-serde-tests branch from 0b0a155 to 63af43a Compare September 9, 2025 23:50
huitseeker and others added 2 commits September 9, 2025 20:12
* Keys in library exports map did not match the name of the actual
  export itself
* Re-computing of the library digest was done differently than 
  Library::new
@bitwalker bitwalker force-pushed the huitseeker/add-serde-tests branch from 63af43a to 550f043 Compare September 10, 2025 00:30
@bitwalker bitwalker merged commit 0b523da into bitwalker/custom-sections Sep 10, 2025
10 checks passed
@bitwalker bitwalker deleted the huitseeker/add-serde-tests branch September 10, 2025 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog This PR does not require an entry in the `CHANGELOG.md` file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants