-
Notifications
You must be signed in to change notification settings - Fork 260
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
Type Parameters in Automatically Generated Tests #2227
Conversation
@tbean79, does this seem to address the issue? |
@Dargones Indeed, this PR solves both issues, I was able to run generated tests in both Block and Path mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job solving two problems at once !
At least update the release notes, but otherwise, I have a few clarifying questions to understand your solution.
@@ -129,10 +147,11 @@ public class TestMethod { | |||
return $"map[{string.Join(", ", mappingStrings)}]"; | |||
case var arrType when new Regex("^_System.array[0-9]*\\?$").IsMatch(arrType): | |||
break; | |||
case var _ when (!variable.Value.StartsWith("(") && variable.Value != "null"): | |||
return "DATATYPES_NOT_SUPPORTED"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain me why datatypes are not supported? What was happening before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have code that supports algebraic datatypes on my fork and can make another PR to add them but for know I just wanted to make it easier for a potential user to figure out why test generation might not work for them. Let me know if I should remove this case from this PR / make another PR specifically for datatypes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To answer your second question, datatypes were never supported (as per this README) and would potentially lead to unspecified behavior. Now the behavior is specified even though the support is not there yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks, It's ok to keep this case in this PR I think. I'm happy to have a look at your other PR about datatypes.
Co-authored-by: Mikaël Mayer <[email protected]>
Thank you, @MikaelMayer! Let me know if there is anything else. I could also bundle support for datatypes with this PR or make a separate PR about them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thanks!
Fixes #2222 and #2221 by adding support for type parameters to counterexample extraction and test generation.
Test generator will replace all type variables with
int
. This should be a valid solution, I think, because all permitted type parameter characteristics apply toint
(i.e. it is equality-supporting, auto-initializable, nonempty, and non-heap based)Regarding the example in the linked issues, the tool will now generate the following:
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.