rafaqz / Interfaces.jl

Macros to define and implement interfaces, to ensure they are checked and correct.
MIT License
72 stars 4 forks source link

Get rid of vector of test objects #22

Open gdalle opened 9 months ago

gdalle commented 9 months ago

I feel like a single test object would make the code cleaner, eg. by removing the need for a wrapper. The user can always make the for loop themselves

rafaqz commented 9 months ago

By wrapper you mean [ ] ?

You're right, currently it's kind of annoying.

The reason it is like this is because the vector was an input to the@implements macro. That made the objects available in a function so we could test at compile time and later build combinatorial interface tests. We sadly dont have that anymore.

Progressively this packake is getting less automated and concise, and less powerful, and Im wondering if that was a mistake.

We should probably at some stage go back having that vector of objects attached to the type via a function, then we could use it programmatically to test other interfaces.

gdalle commented 9 months ago

But I thought you agreed to that change because it allowed the interface user to define that vector of objects, instead of the interface writer? That's really important in my view

gdalle commented 9 months ago

And btw, allowing multiple arguments in an interface is quite a power boost, so it's not all doom and gloom

rafaqz commented 9 months ago

But I thought you agreed to that change because it allowed the interface user to define that vector of objects, instead of the interface writer? That's really important in my view

The person who writes @implements (lets call them the interface implementer) always provided the objects. The interface writer defines @interface - which never had objects.

I allowed the change because the objects are only needed in the tests now and not actually used in @implements, and because over time I had forgotten the reasons I had for them originally, the potential to use them for large scale testing of interface ecosystems. Which I think would be quite powerful.

And btw, allowing multiple arguments in an interface is quite a power boost, so it's not all doom and gloom

Absolutely! Your PRs have been great, Arguments is a solid improvement.

But we do have less information available at compile time for types that implement the interface, and less concrete proof that the interface is tested - than we originally had when I first wrote it.

At some point we will need to have a little discipline to hang on to or add things that on the surface seem annoying but actually have broader potential in the long term.

gdalle commented 9 months ago

I agree that it does seem a bit silly to be able to declare the interface implemented if no tests pass. So in theory, all it takes is to move Interfaces.test back inside Interface.@implements?

gdalle commented 9 months ago

Related:

gdalle commented 9 months ago

This issue was initially about dropping the TestObjectWrapper shenanigans. But I guess if the test objects go back to @implements the option to provide several at once is important, because there can only be one macro call

rafaqz commented 9 months ago

Yeah, it requires reverting half the changes that have been made 😅

@implements needs objects to test, and to output a function that runs tests during precompilation.

There is a point to be made about not having the interface happen in precompile, and Im sure enforcing it will annoy some people.

Maybe we can add a keyword to the macro to not run tests, like @implements SomeInterface :untested, that would make the status of the implementation clear to everyone that reads it, and be available at compile time to other packages in e.g. istested(Interface, obj)

rafaqz commented 9 months ago

Oops was writing when you posted.

Yeah I think TestObjectWrapper was required for dispatch back when it was used in @implements.

rafaqz commented 8 months ago

Either way, I'm thinking youre right, it would be good to remove the requirement to have a vector, but to also allow it

gdalle commented 8 months ago

Not sure how to achieve that without the ugly wrapper

rafaqz commented 8 months ago

At least its a hidden ugly wrapper...