Change serialization

[bartekleon]

I was looking at the interfaces and I think we could use JSON.stringify instead of serialize. E2E tests pass. Unit won't for now since we are testing File. Although I think it is possible to solve it too. Since the only place it can occur is "CallMessage" in args (its any type), so we could just map through args, find if there is File instance, serialize it, and JSON.stringify entire object.
Deserialization would be opposite, JSON.parse and then map serialize on args

[bartekleon]

Edit: Looking further has shown that we actually are not using any non serializable type in processes, so JSON.stringify / parse are enough to handle it. Based on this, I removed unit tests which were testing functionalities/cases which were not used in our code base.

TODO:

• remove surreal dependency
• check if there is no code dependant on that / dead paths
• optional: make types more specific / reduce size of JSON to parse / stringify

[nicojs]

Thanks for taking a look at our serialization.

Using JSON.stringify works in most situations, but there are situations where it doesn't. For example, if a user configures a custom karma configuration inside a stryker.conf.js file with some more complex stuff.

module.exports = {
    karma: {
        config: {
            proxyReq: function(proxyReq, req, res, options) {
              proxyReq.setHeader('Referer', 'https://www.example.com/');
            },
            client: {
              mocha: {
                grep: /fooComponent.*/
              }
           }
        }
    }
}

See https://karma-runner.github.io/5.2/config/configuration-file.html. The same goes for other test runners.

This is sort of a hidden feature, so we might be able to get rid of this feature (obviously our e2e tests don't have anything like this). However, I feel like we might regret removing this at some point in the future.

We could also opt for using serialize-javascript, which is widely used. We're using it in mocha as well, to serialize state in --parallel mode.

[bartekleon]

Uhh.... could we try at least making this as an option? Like making option:

{
  'files': ...,
  'testRunner': ...,
  'advanced': {
     'serialization': true/false
  }
}

and just making: const serialization = config.advanced.serialization ? surrial : JSON.stringify,
Something like this.
Honestly I see no difference between serialize-javascript and surrial. Their basic behaviour is the same, which adds up to the same issues :/

For now I would really like to see the difference in performance between these 2 solutions, and then we could see if we even need this or advanced serialization.

To sum up: Closing for now. Ill wait for flamegraph (either made by me or Latikna), and then we can have debate if there is a need for adding more options :)

[nicojs]

Honestly I see no difference between serialize-javascript and surrial. Their basic behaviour is the same, which adds up to the same issues :/

Yes. It's more to improve maintenance than improve performance 🤷‍♂️