-
Notifications
You must be signed in to change notification settings - Fork 343
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
"Sealed Class and Interface" feature of "OpenJDK-17" on JPF/java-17 #485
Comments
Looks good; provide some comments that give a rationale behind the chosen class hierarchy (what aspects of sealed/non-sealed class behavior is tested by which test). |
Sealed Class Tests
Sealed Interface Tests
|
Is there a need to have both Circle and Square in the class hierarchy? They seem to test the same thing. I also think that floating point equivalence can be problematic (e.g., on M chips that don't use strict_fp by default). Can't we remove Circle? |
We can remove Circle . To make symmetry, should I remove Circle from "sealed class" Test too ? |
Yes, you can remove that class everywhere. It is good to test software thoroughly, but if tests are completely redundant, removing them now reduces the burden for code review and maintenance. Here, Circle acts in the exact same way as Square; only the formulas are different. That aspect is not relevant for sealed/non-sealed/final classes as a feature. |
Should create a PR now ? |
Yes, please do so. |
Thanks! |
@cyrille-artho
@pparizek
Sealed Class Tests
All the above
Tests
passes.Sealed Interface Tests
All the above
Tests
passes.The text was updated successfully, but these errors were encountered: