Implement System Environment Extension

[LeoColman]

This commit implements the System Environment Extensions. With some extension functions and a listener, this commit enables System Environment Override by users, something only possible via reflection.

An special attention is needed to the system environments, as they're susceptible to race conditions. This was specified in the documentations.

This commit also fixes a small issue with SystemProperties extension, in which they wouldn't accept a suspended function. A test for that behavior was added.

This commit also renames the tests package, to stop stacktrace filtering, so that the error is clear whenever one of these tests fail.

Implements #623

[LeoColman]

@sksamuel
Initial implementation. Tell me what you think about it so far

[sksamuel]

We can't use suspend in the extensions.
If you do this:

suspend fun <T> withSystemProperties(props: List<Pair<String, String?>>, thunk: suspend () -> T): T {
    return withSystemProperties(props.toMap(), thunk)
}

Then you can't do this use this in another lambda.

[LeoColman]

If we don't let them suspend, we won't be able to use them in tests. I caught this scenario when trying to write ShouldSpec. Could you give me an example of what what would be a bad case to use it this way?

In ShouldSpec I tried something like:

"Foo" {
    withEnvironment("Boo", "Bar") {
        should("FAR") {

        }
    }
}

[sksamuel]

I use them inside the tests, not outside.

[sksamuel]

Try doing, collections.forEach { withSysProp { ... } }

[LeoColman]

What if we make them inline?
Similar to how it is for shouldThrowAny and similars?

[sksamuel]

If you have a suspend function, and you pass it into a non suspend lambda, the compiler can't reason about when it will be executed. Even if the lambda is currently inside a coroutine scope, there's no saying when that lambda will be invoked.

[LeoColman]

I think that shouldThrow works nice in this sense

[sksamuel]

I don't understand ?

[LeoColman]

Look at the implementation of `shouldThrow`. It uses inline and enables suspended functions, because they are inlined inside another suspend function. Maybe we can do the same with extensions? I'll test it Em sex, 1 de fev de 2019 22:19, Stephen Samuel <notifications@github.com escreveu:

…

I don't understand ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#633 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABgRIx--byIC-DxM7clg7FDZdCk5vS0Aks5vJNmrgaJpZM4afGOb> .

[sksamuel]

Oh I see. Clever.

…

On Sat, 2 Feb 2019 at 00:30, Leonardo Colman Lopes ***@***.***> wrote: Look at the implementation of `shouldThrow`. It uses inline and enables suspended functions, because they are inlined inside another suspend function. Maybe we can do the same with extensions? I'll test it Em sex, 1 de fev de 2019 22:19, Stephen Samuel ***@***.*** escreveu: > I don't understand ? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > < #633 (comment)>, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABgRIx--byIC-DxM7clg7FDZdCk5vS0Aks5vJNmrgaJpZM4afGOb > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#633 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAtZGpJHxEK8nczE9sLOpeVNcumYtr5Rks5vJNw6gaJpZM4afGOb> .

[LeoColman]

@sksamuel I think this is the version I'm comfortable with.

[LeoColman]

I'm trying to verify why app veyor is failing. I think it's resetting my environment

[LeoColman]

@sksamuel
One thing I"m in doubt:
Do we need a version for each scope? I was thinking that maybe TestLevel would suffice, as it can also be used in ProjectLevel.

Ok, the function will be called more times than it should, but this will avoid confusion, no?

[LeoColman]

(This is the reason I didn't add the others)

[LeoColman]

@sksamuel SystemEnvironmentProjectListener added. However, I have no idea how to test it. It would dirty any other tests, and make them too coupled

[sksamuel]

In the past I've done this by making a new sub module. Like I did for parallelisation.

[LeoColman]

I don't know if there's a problem to dirty context with a very specific variable tho... For instance, just a foo in the system environment.

Do you think it's too much of an issue?

[LeoColman]

I added the test while dirtying the context.
Tell me if you're too much against it, and I'll create a module if necessary.

[LeoColman]

Oh, I just saw that you said this test is not completely necessary. I'll remove it, as I believe that this kind of behavior was already tested in the other functions, and project listener execution is also already tested.

[LeoColman]

@sksamuel
Are you ok with these changes now?

[sksamuel]

Yes looks good.

…

On Tue, 12 Feb 2019 at 13:04, Leonardo Colman Lopes < ***@***.***> wrote: @sksamuel <https://github.com/sksamuel> Are you ok with these changes now? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#633 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAtZGjngbYPfg2nzIoRbA1KLYnXuJ76iks5vMrvjgaJpZM4afGOb> .

[LeoColman]

Great. Merging this and doing the same to SecurityManager extension