ballerina-platform / ballerina-spec

Ballerina Language and Platform Specifications
Other
168 stars 52 forks source link

Improve mocking in test framework #619

Open jclark opened 3 years ago

jclark commented 3 years ago

I think we can improve on the current approach to mocking in the test framework

https://github.com/ballerina-platform/ballerina-spec/blob/master/test/test-framework-spec.md#mocking

I will put some ideas in this issue.

jclark commented 3 years ago

I find the terminology of "mock object" and "mock function" not quite right. A "mock function" is not doing the same thing to a function that a "mock object" is doing to an object. I would see "mock function" more as a mock module: you are creating a mock module by overriding specified functions in an existing module, and then running this module using the module in place of the existing module. This is similar to what a mock object is doing to an object: you are creating a new object by overriding specified functions of an existing object. (There's a strong parallel in Ballerina between objects and modules.)

In both cases, there are two possibilities for what to do if the mock does not specify an override: panic or use the existing implementation.

jclark commented 3 years ago

Another way to approach this is that we have:

Mocking a function means you provide a mock definition for a function exported by a module in another function; and this mock definition is used in place of the real definition. Mocking a class means the same but for a class rather than the function, and the mock class definition does not have to explicitly provide definitions for all the methods in the class. These can then work in a reasonably uniform way.

jclark commented 3 years ago

For a mock function definition, can we simply do something like this?

// Provide a function that can be used instead of m:foo.
@test:mockFunction { replaces: m:foo }
function mockFoo() {
}

Then there would need to be a way to control which mock functions are in effect for which tests. Could this be done declaratively, e.g. by grouping the mock functions into sets and saying which sets of mock functions are in effect for which tests?

jclark commented 3 years ago

I can also envisage a langlib function for dynamic object creation. Something like:

public function dynamicNew(record {} r, typedesc<object> t = <>) returns t|error;

This would create an object of type t, with the implementation of each member given by the fields of r:

sanjiva commented 3 years ago

The above would be awesomely powerful .. not just for testing/mocking but for creating dynamic objects using closures as member functions.

sameerajayasoma commented 3 years ago

This lang lib function gives the test framework to mock an object type. However, I believe that this is a low-level function that may not be exposed to test framework users.

We need to provide some high-level utilities for developers to mock classes. Here are some use cases.

type Foo object {
   function get(int i) return string;
   function put(int i, string s);
};

To create a more intuitive API to achieve something like above, we need higher-level utilities in the test module.

sanjiva commented 3 years ago

Distinct objects will be hard to mock ..

jclark commented 3 years ago

The approach of using an annotation on a class definition to say that it is a mock for another class will be hard to make work when the class is distinct. But I think the dynamicNew approach will work with distinct, provided we allow it to work with type names that are defined by class definitions as well (since classes are types+init method).

However, this means we need to make it deal with non-public members. If dynamicNew is applied to a type in another module, then it should be able to specify only implementations for public members. Non-public methods should panic, as should access to non-public members (this will only happen if you mix mock and non-mock implementations of the same module).

jclark commented 3 years ago

@sameerajayasoma I think dynamicNew handles your use case intuitively:

Foo x = object:dynamicNew({get: i => i == 10 ? "ten": ""});

I am relying on the compiler to get the right contextually expected type for the get member, so the type doesn't need to be respecified.

hevayo commented 3 years ago

We went through the object mocking use cases with dynamicNew and it seems to be able to handle all of them. But we do find the following limitations compared to existing API.

  1. Once you create a mock object with dynamic new you will not be able to change the behavior of the mock object between test cases.
  2. In the dynamic new function do you need to match the function signature to the actual function type in the object ? Ie. in the following case should the get function match original function signature. If so, writing a mock object will be a bit of a hassle since you need to go through the APIDocs ( Can be improved by LS completions ).
    http:HttpClient x = object:dynamicNew({
    get: function (string url, http:RequestMessage message, http:TargetType targetType) 
                   returns http:Response | http:PayloadType | http:ClientError {
        http:Response response = new();
        return response;
    }
    });

    The current object mock api address above cases by allowing you to change the mock object behavior without needing to create a new mock object. Also you do not need to specify the full function signature if you just want to return a value.

For mocking functions we can use annotations to provide a mock function. We already implemented it before we changed to the new function mock API. And as mentioned the issue is to change the behavior between test cases. One option is to provide a controller API

test:FunctionController fc = new();

@test:mockFunction { replaces: m:foo }
function mockFoo() return string {
    return fc.respond(param);
}

fc.return(“response”);
// test a valid response
fc.return(error(“error string”));
// test error scenario 
jclark commented 3 years ago

Can't we do this?

http:HttpClient x = object:dynamicNew({
    get: (url, message, targetType) => new http:Response
});
hevayo commented 3 years ago

Yes that looks nice can we refine it further since message and targetType are optional

http:HttpClient x = object:dynamicNew({
    get: (url) => new http:Response
});
jclark commented 3 years ago

Don't think that will type-check. Optional parameters are optional for callers not for subtypes.