Selenide: Thread safe global configs

[admizh]

We run tests in different threads, frequently threads modify global configuration. Is it possible to use my implementation configurations class? Now this is not possible because you uses static fields in config class and it is not thread safe.
If you can create an interface+provider for the configuration will be comfortable and will allow users to modify the behavior of the test in runtime. But it will result in many changes in the existing user code. It would be good to announce it now and commited in selenide 3.0

[asolntsev]

@admizh It's not a bad idea at all. But.. I guess it would require too many changes. We cannot force all Selenide users to modify all their tests.

Though, it would be acceptable if we only need to modify Selenide code itself.

[asolntsev]

@admizh Hi!
We discussed this issue with @yashaka and agreed that it would be possible.
Look for issue https://github.com/codeborne/selenide/issues/354, it's about the same.
I am going to implement it at some moment.
(just for your information)

[admizh]

Great news!

[jane-ryabchenko]

It seems to me that it's impossible do fix this problem in place without breaking backward compatibility.
I can propose following scenario:
1) Create a new ConfigurationOld class, mark it as @Deprecated
2) Make Configuration class to extend ConfigurationOld
3) Move all static parameters from Cinfiguration to ConfigurationOld (for backward compatibility)
4) Create nested class Configuration.ConfigurationContainer and declare a method ConfigurationContainer ConfigurationOld.get() which will return an instance of ConfigurationContainer from ThreadLocal (to make sure that each thread works with it's own version of configuration.
5) For each static variable from ConfigurationOld create a non-static variable in ConfigurationContainer with the same name/type/doc. Mark static variable as @Deprecated and add a comment with a link to a new non-static config variable.

Eventually all old-style static variables will become deprecated as not thread safe and users will migrate to a newer configuration after a while, especially if we will add new configuration parameters to the new ConfigurationContainer only.

[asolntsev]

@jane-ryabchenko I agree, we will do something like this.
But I suggest a little bit another implementation:

1. Leave Configuration class as is. Users who don't parallelize tests, or parallelize with processes (like me), will not need to change anything.
2. Create another ThreadSafeConfiguration class that will take all its initial values from Configuration. It will allow users to set all default values in Configuration and customize them on demand in ThreadSafeConfiguration instances.

[jane-ryabchenko]

Got it. I see two options here

1) When thread calls ThreadSafeConfiguration.get for the first time, we
will copy current values from Configuration as initial values for
_every_ property.

2) We creating a thread local copy of each of the property from
Configuration only when thread tries to change its value.

Wdyt?
On 11-Sep-17 14:27, Andrei Solntsev wrote:
>

@jane-ryabchenko https://github.com/jane-ryabchenko I agree, we will
do something like this.
But I suggest a little bit another implementation:

1. Leave |Configuration| class as is. Users who don't parallelize
   tests, or parallelize with processes (like me), will not need to
   change anything.
2. Create another |ThreadSafeConfiguration| class that will take all
   its initial values from |Configuration|. It will allow users to
   set all default values in |Configuration| and customize them on
   demand in |ThreadSafeConfiguration| instances.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/codeborne/selenide/issues/252#issuecomment-328663679,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AU8sMX07y4uaaFFY0CWtGWtB1Ff0n1okks5shaXKgaJpZM4G0pLo.