Skip to content

Fix: Add tests for extensions's signatures #17133

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.

Sign up for GitHub

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

Jump to bottom
Merged
KacperFKorban merged 2 commits into from
Apr 3, 2023
Merged

Fix: Add tests for extensions's signatures #17133

KacperFKorban merged 2 commits into from
Apr 3, 2023

Conversation

@Dedelweiss
Copy link
Contributor

@Dedelweiss Dedelweiss commented Mar 20, 2023 •
edited
Loading

In this PR, I added new tests examples for extensions's signatures.
To do that I did multiple changement :

SignatureTest

  • Added Extension for expected and actual signatures
  • There needs to be a unique name for each signature, but extensions don't have one like defs. So for now I have the signature itself as a title (can change if better idea)
  • Added "extensions" for member to test

ExtensionParam

  • I changed the extensions so that they are all unique and test different cases.

TranslatableSignaturesTestCases

  • Added a filterNot for ExtensionMethodSignature class. Too many duplicate extensions, it creates an error and I think it's not mandatory to test the extensions on this file too.

Fixes: #17108

@Dedelweiss Dedelweiss force-pushed the add_tests branch 4 times, most recently from 08f26bd to c82af99 Compare March 22, 2023 15:52
@KacperFKorban KacperFKorban mentioned this pull request Mar 23, 2023
Closed
@Dedelweiss Dedelweiss force-pushed the add_tests branch 3 times, most recently from 23cae33 to c15bdfc Compare March 23, 2023 16:22
@Dedelweiss Dedelweiss changed the title WIP: Add tests for extensions's signatures Fix: Add tests for extensions's signatures Mar 23, 2023
@Dedelweiss Dedelweiss marked this pull request as ready for review March 23, 2023 16:35
ckipp01
ckipp01 reviewed Mar 27, 2023
View reviewed changes
@ckipp01 ckipp01 requested a review from Florian3k March 27, 2023 05:48
ckipp01
ckipp01 reviewed Mar 30, 2023
View reviewed changes
KacperFKorban
KacperFKorban reviewed Mar 30, 2023
View reviewed changes
Copy link
Member

@KacperFKorban KacperFKorban left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

Overall looks good, I added some comments.

@Dedelweiss
Copy link
Contributor Author

Dedelweiss commented Mar 31, 2023

The corrections are done, thank you very much for your feedback @ckipp01 @KacperFKorban !

@Dedelweiss Dedelweiss force-pushed the add_tests branch 2 times, most recently from 0ce1120 to 11900fa Compare March 31, 2023 15:39
@Dedelweiss
Added some corrections
e940421 
- Correct typo in SignatureTest file
- Correct from the feedback
@Dedelweiss Dedelweiss requested a review from KacperFKorban April 3, 2023 13:54
ckipp01
ckipp01 approved these changes Apr 3, 2023
View reviewed changes
Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

This looks good on my end. Thanks @Dedelweiss!

KacperFKorban
KacperFKorban approved these changes Apr 3, 2023
View reviewed changes
Copy link
Member

@KacperFKorban KacperFKorban left a comment

Choose a reason for hiding this comment

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

LGTM

@KacperFKorban KacperFKorban merged commit b0ad1e1 into scala:main Apr 3, 2023
@Dedelweiss Dedelweiss deleted the add_tests branch April 4, 2023 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ckipp01 ckipp01 ckipp01 approved these changes

@KacperFKorban KacperFKorban KacperFKorban approved these changes

@Florian3k Florian3k Awaiting requested review from Florian3k

Assignees

@Florian3k Florian3k

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Scaladoc: add tests for extension signatures

4 participants

@Dedelweiss @ckipp01 @KacperFKorban @Florian3k