JuliaTesting / Mocking.jl

Allows Julia function calls to be temporarily overloaded for the purpose of testing
Other
62 stars 14 forks source link

Switch Mocking over to using Cassette #53

Open omus opened 6 years ago

omus commented 6 years ago

Would make the new version of Mocking which uses Cassette only work on Julia 0.7. I expect this work to start once all the major Cassette issues are resolved around the Julia 0.7 release.

simonbyrne commented 5 years ago

Hey, I'd be interested in helping out with this. Have you had any thoughts/designs?

The simplest thing I can think of is defining a macro which wraps a test suite in an overdub, i.e.

@testset_overdub ctx begin
    @test a == b
end

would be equivalent roughly to

@testset begin
    overdub(ctx, () -> begin
        @test a==b
    end
end
omus commented 5 years ago

I like the idea of setting up test set that allows you to pass in a context.

For the initial design I was planning on keeping @patch around which would primarily just convert method definitions into overdub definitions. One difference however would be that I've found that it can be nice in test suites to re-use the same patch in combination with other patches so we probably need to store a representation which we can apply to multiple Cassette contexts.

I also was toying with some value-based dispatch which would allow us to make patches like +(1, 2) = 4 but mostly that was just for extra convenience.

omus commented 5 years ago

Unfortunately, I don't have much free time to do this work at the moment and I expect I may get the time to do this work closer to December.

oxinabox commented 5 years ago

I also was toying with some value-based dispatch which would allow us to make patches like +(1, 2) = 4 but mostly that was just for extra convenience.

That is what https://github.com/oxinabox/ExpectationStubs.jl does.

(Which I have still not tested how well it mixes with Mocking.jl, or Cassette, having not gotten around to actually using either.)

oxinabox commented 5 years ago

I like the idea of setting up test set that allows you to pass in a context.

I am annoyed that it is not possible to do.

@testset OverDub(ctx) "title" begin
#...
end

It might be able to be made to work, if OverDub(ctx) needs to return a constructor for an AbstractTestset, and do the setup stuff fopr the overdubing, and have a finish method that does the teardown for overdubbing. but it is awkward.

(My general finding is setting a custom testset actually never lets you do anything much that is useful. The configuration points it makes available are not the ones you want to configure)

oxinabox commented 5 years ago

I now have proof of concept code for this. Following the current @patch, and apply(fn, patch) API.

It is only about 70 lines long, and is working. It abuses a fair bit of @eval. My current proof of concept doesn't support kwargs because https://github.com/jrevels/Cassette.jl/issues/48

Major breaking change is that it isn't name based, it is value based. So you can't path things without first importing them, and you can't patch functions that are local to a function.

I think there might be a way to change that, but I am not 100% sure. OTOH it solves #16 for free (I think).

omus commented 4 years ago

It's about time I gave an update on this work. Back during the JuliaCon 2019 I reviewed https://github.com/invenia/Mocking.jl/pull/60 and did some experimentation with Cassette based Mocking. That experimentation led to https://github.com/invenia/Mocking.jl/pull/63 which cleaned up the code base significantly and made it easier to utilize Cassette.

Unfortunately, my experimentation discovered that when exceptions are raised in overdub calls there is a significant performance penalty. My testing uses a branch called cv/cassette-experiment which allows the code injection method to be controlled with an environmental variable. I also added an environmental variable which revises how the tests are written to identify the ideal performance possible:

Julia Version Injection Runtime (secs)
1.0.5 macro / rewritten 6.602726
1.0.5 macro 6.619183
1.0.5 codegen / rewritten 8.622261
1.0.5 codegen 28.206396
1.0.5 static / rewritten 22.991566
1.0.5 static 204.327013
1.3.2-pre.0 macro / rewritten 5.720870
1.3.2-pre.0 macro 5.395636
1.3.2-pre.0 codegen / rewritten 7.256142
1.3.2-pre.0 codegen 23.731426
1.3.2-pre.0 static / rewritten 22.231779
1.3.2-pre.0 static 108.637313
1.4.0-DEV.672 macro / rewritten 5.773347
1.4.0-DEV.672 macro 6.498944
1.4.0-DEV.672 codegen / rewritten 7.983218
1.4.0-DEV.672 codegen 21.377317
1.4.0-DEV.672 static / rewritten failure with Cassette
1.4.0-DEV.672 static failure with Cassette

Basically, there is a 3-4x reduction in performance when an exception is raised. This delay makes test driven development quite a bit more annoying especially when multiple exceptions are raised by the tests. At this point I could add optional support for Cassette in place but I would very much like to get the performance closer to that of using the @mock macro.

omus commented 4 years ago

It was mentioned to try using @inline which helped somewhat:

Julia Version Injection Runtime (secs)
1.0.5 macro / rewritten 5.174785
1.0.5 macro 5.438823
1.0.5 codegen / rewritten 6.871501
1.0.5 codegen 24.932569
1.0.5 static / rewritten 19.727543
1.0.5 static 186.011043

Unfortunately the speed difference between macro and codegen (with no re-writing) is more than I would like.

oxinabox commented 4 years ago

Hmm, interesting. I guess natural follow up is to workout how to make the "rewritten" moving of tests not required.

MasonProtter commented 4 years ago

I apologize if I'm spamming, but this may be a good test case for SpecializeVarargs.jl if you apply something like specialize_vararg 6 to your overdub and get_alternate method definitions.

One wrinkle is that SpecializeVarargs actually depends on Mocking for it's splitdef and combinedef methods, so if you want to try it out, you'll probably just have to copy-paste the macro into Mocking.

MasonProtter commented 4 years ago

Okay, SpecializeVarargs.jl now depends on ExprTools thanks to @omus and is tagged, so it should be useable for the Cassette branch now.