Skip to content

Attempt to fix tests for 'repr' feature - #1583

Open
fnimick wants to merge 1 commit into
juhaku:masterfrom
fnimick:repr-enum-tests-broken
Open

Attempt to fix tests for 'repr' feature#1583
fnimick wants to merge 1 commit into
juhaku:masterfrom
fnimick:repr-enum-tests-broken

Conversation

@fnimick

@fnimick fnimick commented Jul 30, 2026

Copy link
Copy Markdown

Note that the tests do not pass as is, as the api docs generated for a 'tagged repr enum' do not appear to match serde_repr's runtime behavior, as validated inline in the test.

Please do not merge!

Note that the tests do not pass as is, as the api docs generated for
a 'tagged repr enum' do not appear to match serde_repr's runtime
behavior, as validated inline in the test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please insta accept the snapshots and commit the result. .new is just the proto-snapshot and needs to be renamed first

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is the one where the generated openapi doesn't match the serde_repr runtime behavior, so the snapshot output here is wrong (potentially). That's why I left it in this state.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@CommanderStorm I wasn't sure what to do here in the case where I'm not sure whether an existing behavior is a bug or not, as I've never used repr enums but the output seems to not make sense. Should I open an issue?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if the behaviour is wrong, that would be best.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I forgot that I already did: #1584

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.

2 participants