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

Fixes some causes for type ambiguity in tests. #474

Merged

Conversation

Superhepper
Copy link
Collaborator

  • Many of tests uses macro to create the test code and in this test code it is common practice to use into or try_into conversions. But this can cause ambiguities later on if new types introduced implements traits that previously only was implemented by one type. So in order to avoid this kind of ambiguity some of the conversions have been changed. This only a first step of changes that probably needs to be done. This fixes the errors that occurs if one tries to add serde_json to the cargo.yaml which is probably needed in order to be able to write tests for Add serde to Public and Private #466.

- Many of tests uses macro to create the test code and in this
  test code it is common practice to use ```into``` or
  ```try_into``` conversions. But this can cause ambiguities later
  on if new types introduced implements traits that previously only
  was implemented by one type. So in order to avoid this kind of
  ambiguity some of the conversions have been changed. This only
  a first step of changes that probably needs to be done. This fixes
  the errors that occurs if one tries to add ```serde_json``` to the
  ```cargo.yaml``` which is probably needed in order to be able to
  write tests for parallaxsecond#466.

Signed-off-by: Jesper Brynolf <jesper.brynolf@gmail.com>
Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

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

Titanic effort but I like it, especially that some stuff like convert_to_u32_test will now give better error messages 👍

@Superhepper Superhepper merged commit aa68349 into parallaxsecond:main Nov 30, 2023
10 checks passed
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

3 participants