nohwnd / Mock

Prototyping scope aware mock
MIT License
5 stars 2 forks source link

Understand how the scoping works #2

Open nohwnd opened 7 years ago

nohwnd commented 7 years ago

(let's move our discussion here from the PR, please)

@alx9r Again thanks a lot for the tests provided in #1 I was able to pass all of them easily. (Well the last one took me few minutes, before I noticed there is a typo in it.)

What I needed to do was just to realize in which module I will call the function, and mock it there. I also had to make sure the mock remains in scope.

To me it seems to behave very consistently, and even the difficult cases I was able to fix and understand easily. So I am pretty happy about it.

What I don't like much is that in most of my fixes I am running the test code in a different scope than it was originally run. This is not desirable for testing the public surface of the module, what I want to do instead is to replace calls within modules with mocks, but still run my tests, in the top level scope.

The simplest way to achieve that was to store the top level exection state, mock the module functions, and then run the tests in the stored state. Pester does something similar to avoid overwriting parent variables with command parameters, I am just moving that to different level. The syntax is not very nice, but it's a starting point.

alx9r commented 7 years ago

@nohwnd

What I don't like much is that in most of my fixes I am running the test code in a different scope than it was originally run. This is not desirable for testing the public surface of the module, what I want to do instead is to replace calls within modules with mocks, but still run my tests, in the top level scope.

I couldn't agree more. I'm pretty sure it's possible to mutate the functions of a loaded module. If that's true, it seems like it would be possible to provide an interface something like New-Mock -FunctionName k1 -Scope j\k which reaches into j\k and changes the function k1 accessible from module j, but does not change the k1 functions accessible elsewhere. And then resets those modules when the test is done.

I also though of a couple more ways of invoking that ought to be tested as well:

  1. invoking an unexported function from a class method of an instance originating inside a module but invoked outside the module
  2. invoking a function with .Invoke(), etc (behavior is different from & in at least some ways per PowerShell/PowerShell#3018.
  3. each of the existing tests in scopeEdgeCases.ps1 involving dynamic modules should also be tested with file-backed modules (there are significant quirks with dynamic modules at least for loading, unloading, and lifetime of functions)

You were wondering about my use of [scriptblock]::create(). Per PetSerAl's example, invoking [scriptblock]::create() creates a scriptblock that is not bound to any module. & {} gets bound to the module in which it was created. That has implications for at least $_. I suspect it also has implications for other variables like $ErrorPreference whose accessibility is affected by scope boundaries. It can't be inferred whether this has implications for whether a mock gets called. The only way I've found to manage this sort of thing is to test all of my assumptions and keep testing them against each update to PowerShell/WMF/.Net. There's no current language spec (as far as I know) that we can rely on for these sorts of things, so all we have is empiricism. That works OK, but it results in a great deal more test code compared with doing similar things in a spec'd language like C# or C++.

Have you thought about accessibility of variables inside mocks? $_, for example, can be passed without involving param() and is affected by scope-ey things. Do you care if the mock doesn't see the same $_ as the real function? Ensuring that would expand the test matrix, but it might be worth it.

nohwnd commented 7 years ago

I'm pretty sure it's possible to mutate the functions of a loaded module.

That is what I am doing I hope. :) I just don't mutate them permanently, which is great because then we don't have to re-load all modules module every time a mock runs out of scope (and re-apply all mocks).

( I am probably over-explaining, but it might be helpful to document my thinking for other people who might see this repo when it goes public. ) How I understand my code is working is that it works the same way this works:

function f { 'i am real' } # <- f
&{
    function f { 'i am shadow' } # <- f1
    f # <- function f is shadowed by f1
        &{
            function f { 'i am shadow' } # <- f2
            f # <- function f is shadowed by f2
        }
}
f # <- function is the original function f

We define a function and shadow it in a different scope. The re-defining a functions shadows the original function. The lifetime of the shadow function is bound to the scriptblock that contains it.

In my code there is one more complication, we can be running each of the scripblocks in a different scope, but luckily PowerShell handles it nicely, and removes the functions once their scopes ends. This gives me exactly the behaviour I need, with very clear scoping of each mock, and very clear "placement" of the mock.

The one thing that was missing is being able to mutate a function in module scope, and then transition back to the top-level scope and test the _public+ interface of our module from there, but the last test shows how to do that.

I will rewrite all the test cases that can be tested purely through their public api is possible to use that approach, to see if everything still works.

I also though of a couple more ways of invoking that ought to be tested as well:

Would it be possible that you wrote test cases for the first two? For the third one (moving to file based modules) I definitely plan to do that, but let's figure out how it works with the dynamic modules, and what edge cases we need to cover, and then start complicating it with file based modules, please.

You were wondering about my use of [scriptblock]::create().

Thanks for the explanation, I will keep it as a reference. Many of our cases are probably extreme edge cases, but even those should behave consistently if the mock replacement is worth something.

Have you thought about accessibility of variables inside mocks?

I haven't thought about variables much in general, but I know I had problems passing $_ around in the Assert module I am writing, so I expect some problems here as well. :) We definitely need to have a good grasp of how variables work to be able to filter in the ParameterFilter. For the MockWith body variables do not seem to be too important, mocks should be stupid, so the most you'd do with the value is print it. There probably will be a whole list of things to test, but $_ should probably be quite high on that list.

I am considering adding a bit more explanation, and publishing this, so other clever people can have a look an add ideas / criticise. I think we've proven that this is most likely idea worth pursuing, so why keep it secret. Agree?

nohwnd commented 7 years ago

@alx9r made some updates, it seems to work really nice. You can nest the blocks that run code in different scopes many times, and then transition back to the script scope and run your tests from there. When each scriptblocks runs out of scope, the variables and functions defined in that scriptblock are gone, no permanent mutation is done. We pretty much do what pester alerady does, but just add returning to the main scope to that.

alx9r commented 7 years ago

I think we've proven that this is most likely idea worth pursuing, so why keep it secret. Agree?

I agree.

alx9r commented 7 years ago

Would it be possible that you wrote test cases for the first two?

I've self-assigned doing this in #3 and #4.

alx9r commented 7 years ago

I am probably over-explaining, but it might be helpful to document my thinking for other people who might see this repo when it goes public.

Your explanations help. I must confess that I've been focused more on finding weaknesses in the behavior than understand how it works. But I think that's why you asked for my help. :)

nohwnd commented 7 years ago

I explain to be able to understand it myself, and it felt like waste to delete it :)

Also I think i might missed that you can do the same thing easier in pester if you use the ModuleName on the mock, instead of using in module scope. Still it needs to manage the mocks, so I should find a way to bind the mock lifetime to the block of It. Hard to explain what I mean exactly without examples. (And also one more rason to keep this just a prototype. The repo is public since few days ago btw :))

alx9r commented 6 years ago

What I don't like much is that in most of my fixes I am running the test code in a different scope than it was originally run. This is not desirable for testing the public surface of the module, what I want to do instead is to replace calls within modules with mocks, but still run my tests, in the top level scope.

I wrote a proof-of-concept that decouples the scriptblock that performs the shadowing alterations from the scriptblock that performs the test. That way neither Invoke-InModuleScope nor Invoke-InScriptScope is necessary for the \shadowing mechanics to work.

nohwnd commented 6 years ago

I think I will need to review what your PoC does one more time :)

alx9r commented 6 years ago

:) I think that's fair. I spent a considerable amount of time understanding yours. ;)

BTW There's a discussion about a supported method for invoking scriptblocks in the global and/or caller SessionState in PowerShell/PowerShell#6147. This private reflection is vulnerable to bucket 4 breaking changes in the breaking changes contract so I've been looking for an alternative better suited for production.

nohwnd commented 5 years ago

@alx9r I am reporting back that after multiple tries I finally understood your example, took only a year :))