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

Ensure hint are set and used for validation of union #185

Closed

Conversation

daddykotex
Copy link
Contributor

No description provided.

expect(result == CheckedOrUnchecked.CheckedCase(CheckedString("foo")))
}

pureTest("Invalid union values fails to parse") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Baccata

I'm not 100% sure I understand the issue because, in bad TDD fashion, I wrote the test after, then checked if the test failed before my fix, but it does not.

to be clear, the test passes w/o the fix, so I'm not sure if it does anything

Copy link
Contributor

Choose a reason for hiding this comment

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

Try moving the constraint from the string shape to one of the alternatives in the union definition in smithy (similarly to how you'd annotate fields in structure shapes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, the issue happens if the annotation is placed directly on a member of the union, the fixes works

@Baccata
Copy link
Contributor

Baccata commented Apr 15, 2022

Superseded by #186

@Baccata Baccata closed this Apr 15, 2022
@daddykotex
Copy link
Contributor Author

daddykotex commented Apr 15, 2022 via email

@Baccata
Copy link
Contributor

Baccata commented Apr 15, 2022

No worries

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