-
Notifications
You must be signed in to change notification settings - Fork 266
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
Add support for Java mocking #2024
base: master
Are you sure you want to change the base?
Add support for Java mocking #2024
Conversation
Add /generateTestBoogie in DafnyTestGeneration
… into testFrameworkCSharp
… into testFrameworkCSharp
Thanks! I saw that part. Given that the feature is growing (to support more languages), I'm wondering whether the whole mocking / testing aspect shouldn't be developed out-of tree. |
Without a stable public API for DafnyPipeline, I don't see how this could be developed from another repository without often breaking. How do you see that working? |
I was hoping we could use this as a way to develop a stable plugin API sufficient for this use case |
My guess is that getting to an API that we would feel comfortable calling stable would require some refactoring. By keeping this consumer in our repository, we can do such refactorings without breaking them. I think we should drive this the other way around, wait for a problem that occurs from having too much in a single repository, and then allocate time to define the API and separate something out to a new repo. |
Got it, thanks! |
Sorry, clicked the wrong button :/ |
Hi Dafny team, Thanks |
Could you resolve the merge conflicts in the PR? |
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.
Glad to see there's interest in this PR @hirataqdees!
@cassidywaldrip - we need to get a clearer view of what the proposed changes are here before we can review, especially the {:synthesize}
semantics changes. There are remnants of other changes in here that are muddying the waters. Can you please update from master and resolve the conflicts?
Hi @robin-aws, I updated from master and resolved the conflicts. Let me know if there are other issues that come up. I'll try to be available to fix them right away. |
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.
Thanks for tidying this up! I've added one comment we should address before I review the rest, and my apologies for not having all the state paged into my head from the last time we discussed it. :) On the other hand, having thought on this for a while may have changed the strength of my opinion on it.
docs/DafnyRef/Attributes.md
Outdated
return one or more fresh or mocked objects. To create a fresh instance of | ||
an object, use the "fresh" parameter (`{:synthesize "fresh"}`). To create a | ||
mocked instance of an object, use the "mock" parameter (`{:synthesize "mock"}`). |
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.
My intention for suggesting the name {:synthesize ... }
for this feature was to keep the semantics as abstract as possible: create an object whose behavior aligns with the specification. Using a mocking framework is just one possible implementation strategy, and one that could change in the future: alternatively you could actually synthesize source code in the target language that obeys the specification.
I'm concerned about committing to the semantics of {:synthesize "fresh"}
, since AFAICT those semantics are to return a new instance of an object without invoking any constructor. Depending on the compilation strategy, that could easily create an object that causes errors/crashes/exceptions when used, rather than the intention (I believe) of producing a valid but underspecified object, whose fields are initialized arbitrarily.
It seems to me that it is only valid to use {:synthesize "fresh"}
if fresh(o)
is literally the only specified post-condition. Could you switch implementation strategies automatically based on that instead? Alternatively, could you stick with the mocking implementation instead, in the name of simplicity? (I recall we discussed this earlier but I can't recall the reason for wanting to make this distinction).
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 updated this branch so that it matches the C# version of synthesize
. I did this by using spies instead of mocks. By using spies, the need for a "fresh" argument is eliminated, as fields can be accessed and updated in a spy (they could not be with a mock). A spy allows for the same customization of behavior as a mock. The difference is that a spy comes with all the functionality of a real object by default, and that functionality can be changed, whereas a mock comes with none of the functionality of a real object by default, so everything must be added.
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.
Excellent, I'm so much happier with that! Thanks for sticking with this for the better outcome. :)
Please do revert this documentation change since it's not needed any more, aside from the Java specific remarks below.
Add DafnyRuntime.Tests to Dafny.sln (dafny-lang#2744)
…to merge-mocking-changes
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 news on not needing "mock" / "fresh" any more! I've done a proper review now and I think we're pretty close here.
(since I can't comment on deleted files) Why was DafnyTests.csproj deleted?
docs/DafnyRef/Attributes.md
Outdated
return one or more fresh or mocked objects. To create a fresh instance of | ||
an object, use the "fresh" parameter (`{:synthesize "fresh"}`). To create a | ||
mocked instance of an object, use the "mock" parameter (`{:synthesize "mock"}`). |
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.
Excellent, I'm so much happier with that! Thanks for sticking with this for the better outcome. :)
Please do revert this documentation change since it's not needed any more, aside from the Java specific remarks below.
Source/DafnyRuntime/DafnyRuntimeJava/src/test/java/dafny/EuclideanTest.java
Outdated
Show resolved
Hide resolved
/// | BOUNDVARS, BOUNDVARS | ||
/// | ||
/// </summary> | ||
public class JavaSynthesizer { |
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.
This generally LGTM, but it seems like there's a ripe opportunity to introduce a common base class for this and the CsharpSynthesizer
class to reduce the duplication. I don't want to further delay this PR now that we're better aligned on the design, but I'd appreciate it if you could time-box a bit of time to refactor - if it becomes a rathole I'd settle for a TODO comment.
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.
Hi! I introduced a common Synthesizer base class and fixed the things you suggested.
LGTM! Just a couple final bookkeeping tasks, if you please:
Thanks so much for the contribution! |
Fixes #
This branch adds support for mocking and the creation of fresh objects to the Java compiler (functionality that already exists for the C# compiler).
Mocking in Java is similar to mocking in C#. The only significant difference between the two at this point is that the Mockito library for Java does not support mocking fields (C#'s Moq does). So, the grammar for mocking in Java is:
S = FORALL | EQUALS | S && S EQUALS = ID.ID (ARGLIST) == EXP // stubs a method call | EQUALS && EQUALS FORALL = forall SKOLEMS :: EXP ==> EQUALS ARGLIST = ID // this can be one of the skolem variables | EXP // this exp may not reference any of the skolems | ARGLIST, ARGLIST SKOLEMS = ID : ID | SKOLEMS, SKOLEMS
Here is an example of using the
{:synthesize}
attribute for creating a fresh method and for creating a mock.The methods annotated with
{:synthesize}
are automatically translated to the following Java:By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.