dart-lang / language

Design of the Dart language
Other
2.68k stars 205 forks source link

Make unit tests a first class citizen #2476

Open bsutton opened 2 years ago

bsutton commented 2 years ago

This issue came out of a discussion around static metaprogramming and some thoughts I've had on unit testing for a while now.

https://github.com/dart-lang/language/issues/1482

I think it's fair to say that unit tests are considered a critical piece in building any application.

The problem is that building unit tests is difficult, as is maintaining them.

For many projects I'm involved in, it's not uncommon for me to spend more time maintaining the unit tests than I spend on making actual code changes.

This seems highly problematic and I believe this is largely the result of the unit tests being treated as a second-class citizen in dart (as it is in many other languages).

We expect a language to have a debug mode, wherein the language allows things to be done in debug mode that the language won't do in release mode.

In the same way that we have a debug and a profile mode, we should have a test mode.

The current package approach (test, mock, dependency injection) is clearly not adequate as writing tests is a significant labour.

Writing unit tests often (always?) requires us to architect our code around supporting unit tests. Whilst some may argue that the discipline of writing unit test can improve our architecture, this is not the same thing as a process that as of today, inherently dictates our application architecture.

There are a number of common scenarios that the language could support in a 'test mode' that would reduce or eliminate the need to re-architecture our code and potentially make writing unit tests quicker and less brittle.

I don't claim to have all the answers as to what this might look like but there are a few obvious starting points

1) private methods. The test mode should allow a unit test to call a private method without any special annotations required on the target library. This should probably extend to third-party libraries.

2) overload static methods I recently wanted to overload the exit() method to stop calls to it from shutting down my unit test. If a singleton provides the required solution then I should be able to choose a singleton.

I should be able to scope these overloads to a test or group. i.e. one unit test overloads the exit() method but another doesn't or does so in a different manner.

3) support for mocking Using the existing mocking frameworks is ugly and requires an unnecessary amount of code.

I should be able to capture and mock an instance creation that occurs deep in the call tree.

Again, we should be able to scope these mocks.

I'm sure there are other ideas and pain points that we could solve with a 'test mode' built into the language.

references

https://en.wikipedia.org/wiki/AspectJ

The java AspectJ library provides some interesting concepts in terms of writing an expression that matches functions or classes and allows their behaviour to be altered.

https://github.com/dart-lang/language/issues/1482

Concepts from dart's forthcoming static metaprogramming combined with additional features when running in test mode might also provide some paths forward.

Summary

Making unit tests a first-class citizen via language support for testing would benefit the entire dart community by: 1) reducing the labour spent on creating/maintaining unit tests 2) remove the need to design architecture that suit the needs of unit tests over and above the applications problem space. 3) potentially increase the use of unit tests by making them easier to build and maintain

Wdestroier commented 2 years ago

I'd prefer if everything was publicly accessible and functions/methods/properties could be reassigned.

Never exit(int code) { ... };

could be interpreted as

Never Function(int) exit = (code) { ... };

and you would be able to change it to a no-operation:

exit = (_) {};

However I don't totally agree that tests should be run in a special mode.

lrhn commented 2 years ago

Overloading static methods is a tough feature.

It should be scoped, because otherwise an override in one part of the program can affect statically resolved functions called in another part of the program. That would heavily impair modular compilation, and prevent any kind of optimization. It also prompts the question of what happens if two different parts of the program tries to modify the same static method.

The general solution to that is first class libraries. Very much non-trivial.

The trivial solution is for each library with static methods to provide their own way to override them. Or for someone to write an abstraction over a set of static methods, and when you only access the functionality through the abstraction, you can change the behavior. (Basically: Stop using static methods.)

Accessing privates from other libraries is probably possible, but also non-trivial. The easy approach is to allow a library to declare itself a "friend" of another (one!) library, and share private member namespace with that library. Probably only allowed for libraries inside the same package. And you can't access privates from more than one library in that way, because that would mean those other libraries also share privacy mangling, and that's too error-prone. Alternatively, there is something more complicated like private imports.

Not sure what "mocking" means here. It sounds like you want to override an object creation (static invocation) to create a mock object, which means it's basically static method interception again (plus some mock to return, but that's an unrelated issue.)

It would be nice to be able to create a mock object entirely at run-time, without having to declare a class for it. That is, a way to create an instance of an interface (or function type), such that all method invocations on the object are forwarded to a callback with a signature like noSuchMethod. Say var foo = Object.createProxy<Foo>((invocation) => ...);. It's currently possible to create such an object, say using class ProxyFoo extends Mock implements Foo {}, but it requires an extra declaration to be added somewhere separate from where it's used. That's bad locality. The advantage of the createProxy in comparison would be that you don't need to write the class. (And maybe we can create a function proxy as well, using Function.createProxy<functiontype>((arguments) => ...). (I'd wait for records before trying that.)

bsutton commented 2 years ago

It should be scoped

I see this as critical for a useful solution.

That would heavily impair modular compilation, and prevent any kind of optimization.

This (if I've understood your point) is why I've suggested that this be an alternate mode. In test mode (like profile mode) we accept an impact on performance in exchange for simplified testing.

The trivial solution is for each library with static methods to provide their own way to override them.

This is what I'm trying to avoid, because in reality, this has proven to be anything but trivial.

I have raised an issue to get exit added to IOOverrides, I'm not expect this to get down any time soon and the same goes for any third party libraries I might be using. This also defeats one of the core principles I'm trying for, in that we shouldn't have to re-archiect our code just so we can write unit tests.

I've been reading up on aspectj and it might provide some solutions.

Aspectj has three concepts: join point - a class/function/method/field advice - action taken at a join point - pointcut - a regex that matches a join point and applies an advice

Actions can be a method that runs, before, after, 'instead of' or when a throw occurs.

A pointcut is global so it applies across the app. I don't love this solution because it requires a whole new declaration but it's still a better solution than the one we don't currently have.

A point cut with standard dependency injection might provide a solution for overloading statics etc.

In the example below I use the Scope dependency injection library but obviously, any DI library would work.

@pointcut  Never exit(int code) 
class ExitPointCut implements PointCut
{
    Never before(int code) { 
    }
    Never after(int code) {
    }
    Never instead(int code)
   {
        // retrieve the injected lambda and call it.
         Scope.use(ExitScopeKey).call(code);
        // exit is a little problematic as it should never return
         throw RuntimeError();
   }
    void onThrows(Exception e, Stacktrace st) 
  {
  }
}

The pointcut would be defined globally (aspectj has some rules for multiple declarations) and then in our unit tests we could use standard DI.

void main()
{
    void test() {
      int exitCode;
       Scope()
           // inject a scoped lambda
           ..value(ExitScopeKey, (_exitCode) => exitCode = _exitCode))
           ..run(() {
               /// some function deep in the bowels of my app calls exit.
                exit(2);
           }
       expect(exitCode, 2);
}

Not sure what "mocking" means here. It sounds like you want to override an object creation Correct, I'm looking to reduce the overhead it takes to create a mock which includes overloading the constructors (which I assume are really just static methods).

I don't mind the idea of createProxy particularly if it could be extended to static functions, static variables and constructors. However, I'm not quite certain how this would be scoped? Edit: i guess the scoping is the same as for a pontcut. Given that tests don't running in parallel (unless in an isolate)

As to privates the pointcut or createProxy should be applicable to private members as well.

because that would mean those other libraries also share privacy mangling I'm not familiar with the details around mangling of private variable names, but in a 'test mode' can the mangling be disabled. In the dart world, we are always compiling against source so I assume we will always have access to the original private names.

bsutton commented 2 years ago

I also wonder if static meta-programming (SMP) can help with the solution.

My understanding is that you can pass arguments to an SMP macro.

Might it allow us to do:


@pointcut(instead: (int code) { 
         Scope.use(ExitScopeKey).call(code);
         throw RuntimeError();
   }
Never exit(int exitCode);

 // intercept the File constructor and change a private variable with the File instance
@pointcut(after: (instance) {
    instance._path = 'some alternate path';
})
Class.File.ctor;

Can you apply an SMP macro to a definition (Never exit(int exitCode) or can it only be applied at the declaration site?

lrhn commented 2 years ago

AspectJ is, effectively, metaprogramming. Because of how Java is designed, they can do a lot of the modifications during compilation, by acting as a compiler for the slightly-extended language, but they can also inject advice at runtime using special class loaders. In the end, it's just generate Java byte code, so it is a kind of metaprogramming on top of Java.

AspectJ is incredibly powerful. It's also nigh unreadable (it's the only other language which has, effectively, implemented Brainfuck's "comefrom" control flow operator). By separating concerns into aspects, you lose locality. Any method called, anywhere, can be wrapped in any number of modifications, which can even choose to not call the original function at all. Your final class C with a concrete method declaration, foo, might not be the method called when doing new C().foo().

Advocates will say that if you use aspects responsibly, they will actually make your code better, and improve your separation of concerns.

(I think the Beta language did that better, because methods were classes, with a body that was executed when an object was instantiated. Calls were instantiations which ran that code, private variables were instance variables of the invocation object, and because of that, your methods could extend a framework class with code that could call inner to invoke the subclass code. That way you could explicitly inherit the wrapper code that AspectJ has to attach from the side, as an around-advice.)

cedvdb commented 2 years ago

I should be able to capture and mock an instance creation that occurs deep in the call tree.

That's the idea behind dependency injection though. If something has to be replaced in a test, who's to say someone else won't have to replace it in another context ? The fact that there is only 2 contexts at one point, (production code & testing) is a bit beside the point isn't it ?

bsutton commented 2 years ago

@cedvdb I do use DI and 'use case' level mocks for unit tests. The problem with DI is that it requires that I own all of the dependencies.

I'm looking for a solution that will work with third party packages as well as my own.