feat(serialize): remove surrial (#2877)

Replace [surrial](https://www.npmjs.com/package/surrial) with `JSON.stringify` and `JSON.parse` to serialize and deserialize the messages between `Checker` and `TestRunner` worker processes.

This has a couple of advantages:
✅ Improves performance (slightly).
✅ Removes a (small) security risk where an [`eval`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval)~ish way was used to deserialize.
✅ Reduces weird behavior in edge cases where you provide functions in Stryker options.
✅ Reduces maintenance load by removing a fringe dependency (that I maintain, but still 😅).

However, this does mean that an undocumented feature disappears (a small, but breaking change).

```js
// stryker.conf.js
module.exports = { 
  karma: {
    webpack: {
      some: /regex/ // 👈 this will be send as `null` to the worker process.
    }
  }
};
```

This is why Stryker will provide an optional warning:

![image](https://user-images.githubusercontent.com/1828233/117358371-5f3c4b80-aeb6-11eb-972b-059d4a8ff7b9.png)

BREAKING CHANGE:  Having a non-JSON-serializable value in your configuration won't be sent to the child process anymore. If you really need them in your test runner configuration, you should isolate those values and put them in test runner-specific config files, loaded by the test runner plugin itself, for example, jest.config.js, karma.conf.js, webpack.config.js.

e2e/helpers.ts

@@ -4,10 +4,49 @@ import { mutationTestReportSchema } from '@stryker-mutator/api/report';
 import { expect } from 'chai';
 import path from 'path';
 import { calculateMetrics, MetricsResult, Metrics } from 'mutation-testing-metrics';
+import { execSync, ExecException } from 'child_process';
+
+interface PipedStdioSyncExecException extends ExecException {
+  stdout: Uint8Array;
+  stderr: Uint8Array;
+  status: number;
+}
+
+export interface ExecStrykerResult {
+  exitCode: number;
+  stdout: string;
+  stderr: string;
+}
+
+export function execStryker(cmd: string): ExecStrykerResult {
+  let result: ExecStrykerResult;
+  try {
+    const out = execSync(cmd, { stdio: 'pipe' });
+    result = {
+      exitCode: 0,
+      stderr: '',
+      stdout: out.toString(),
+    };
+  } catch (err) {
+    const childError = err as PipedStdioSyncExecException;
+    result = {
+      exitCode: childError.status,
+      stderr: childError.stderr.toString(),
+      stdout: childError.stdout.toString(),
+    };
+  }
+  if (result.stdout) {
+    console.log(result.stdout);
+  }
+  if (result.stderr) {
+    console.error(result.stderr);
+  }
+  return result;
+}
 
 export async function readMutationTestResult(eventResultDirectory = path.resolve('reports', 'mutation', 'events')) {
   const allReportFiles = await fsPromises.readdir(eventResultDirectory);
-  const mutationTestReportFile = allReportFiles.find(file => !!file.match(/.*onMutationTestReportReady.*/));
+  const mutationTestReportFile = allReportFiles.find((file) => !!file.match(/.*onMutationTestReportReady.*/));
   expect(mutationTestReportFile).ok;
   const mutationTestReportContent = await fsPromises.readFile(path.resolve(eventResultDirectory, mutationTestReportFile || ''), 'utf8');
   const report = JSON.parse(mutationTestReportContent) as mutationTestReportSchema.MutationTestResult;
@@ -36,7 +75,6 @@ export async function expectMetricsResult(expectedMetricsResult: Partial<Metrics
     if (typeof actualSnippet.metrics.mutationScoreBasedOnCoveredCode === 'number') {
       actualSnippet.metrics.mutationScoreBasedOnCoveredCode = parseFloat(actualSnippet.metrics.mutationScoreBasedOnCoveredCode.toFixed(2));
     }
-
   }
   expect(actualSnippet).deep.eq(expectedMetricsResult);
 }
@@ -71,6 +109,6 @@ export function produceMetrics(metrics: Partial<Metrics>): Metrics {
     totalMutants: 0,
     totalUndetected: 0,
     totalValid: 0,
-    ...metrics
+    ...metrics,
   };
 }

e2e/test/options-validation/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "options-validation",
+  "scripts": {
+    "pretest": "rimraf stryker.log .stryker-tmp",
+    "test:mutation:plugin": "stryker run --fileLogLevel info stryker-error-in-plugin-options.conf.js || exit 0",
+    "test": "mocha --timeout 60000 --require \"../../tasks/ts-node-register.js\" verify/verify.ts"
+  }
+}

e2e/test/plugin-options-validation/stryker-error-in-plugin-options.conf.json → e2e/test/options-validation/stryker-error-in-plugin-options.conf.json

@@ -8,5 +8,8 @@
   },
   "karma": {
     "projectType": "Project type not supported"
+  },
+  "commandRunner": {
+    "command": "echo 'no-test'"
   }
 }

e2e/test/options-validation/stryker-warnings.conf.js

@@ -0,0 +1,9 @@
+module.exports = {
+  myCustomReporter: {
+    filter() {
+    }
+  },
+  commandRunner: {
+    command: 'echo "no-test"'
+  }
+}

e2e/test/options-validation/verify/verify.ts

@@ -0,0 +1,50 @@
+import { expect } from 'chai';
+import { execStryker, ExecStrykerResult } from '../../../helpers';
+
+describe('Options validation', () => {
+  let result: ExecStrykerResult;
+
+  describe('Errors in plugin options', () => {
+
+
+    before(() => {
+      result = execStryker('stryker run stryker-error-in-plugin-options.conf.json');
+    });
+
+    it('should exit with 1', () => {
+      expect(result.exitCode).eq(1);
+    });
+
+    it('should report mochaOptions.spec error', () => {
+      expect(result.stdout).includes('Config option "mochaOptions.spec" has the wrong type');
+    });
+
+    it('should report jasmineConfigFile error', () => {
+      expect(result.stdout).includes('Config option "jasmineConfigFile" has the wrong type');
+    });
+
+    it('should report karma.projectType error', () => {
+      expect(result.stdout).not.includes('Config option "karma.projectType" has the wrong type');
+      expect(result.stdout).includes('Config option "karma.projectType" should be one of the allowed values');
+    });
+  });
+
+  describe('Warnings ', () => {
+
+    before(() => {
+      result = execStryker('stryker run stryker-warnings.conf.js');
+    });
+
+    it('should exit with 0', () => {
+      expect(result.exitCode).eq(0);
+    });
+
+    it('should report about unknown options', () => {
+      expect(result.stdout).includes('Unknown stryker config option "myCustomReporter"');
+    });
+
+    it('should report about unserializable options', () => {
+      expect(result.stdout).includes(' Config option "myCustomReporter.filter" is not (fully) serializable. Primitive type "function" has no JSON representation. Any test runner or checker worker processes might not receive this value as intended.');
+    });
+  });
+});

packages/api/.nycrc.json

@@ -7,7 +7,7 @@
   "checkCoverage": true,
   "reporter": "html",
   "reportDir": "reports/coverage",
-  "exclude": [
-    "testResources/**/*.js"
+  "include": [
+    "dist/src/**/*.js"
   ]
 }

packages/api/package.json

@@ -41,7 +41,6 @@
   "dependencies": {
     "mutation-testing-report-schema": "1.7.1",
     "mutation-testing-metrics": "1.7.2",
-    "surrial": "~2.0.2",
     "tslib": "~2.2.0"
   },
   "devDependencies": {

packages/api/schema/stryker-core.json

@@ -221,6 +221,11 @@
           "description": "decide whether or not to log warnings when a preprocessor error occurs. For example, when the disabling of type errors fails.",
           "type": "boolean",
           "default": true
+        },
+        "unserializableOptions": {
+          "description": "decide whether or not to log warnings when a configuration options are unserializable. For example, using a `/regex/` or `function` in your configuration options.",
+          "type": "boolean",
+          "default": true
         }
       }
     }

packages/api/src/core/file.ts

@@ -1,9 +1,7 @@
-import { Surrializable, surrial } from 'surrial';
-
 /**
  * Represents a file within Stryker. Could be a strictly in-memory file.
  */
-export class File implements Surrializable {
+export class File {
   private _textContent: string | undefined;
   private readonly _content: Buffer;
 
@@ -37,12 +35,4 @@ export class File implements Surrializable {
     }
     return this._textContent;
   }
-
-  /**
-   * This fixes the issue of different File versions not being compatible.
-   * @see https://github.com/stryker-mutator/stryker-js/issues/2025
-   */
-  public surrialize(): string {
-    return surrial`new File(${this.name}, ${this.content})`;
-  }
 }

packages/api/test/unit/core/file.spec.ts

@@ -1,5 +1,4 @@
 import { expect } from 'chai';
-import { deserialize, serialize } from 'surrial';
 
 import { File } from '../../../src/core';
 
@@ -13,19 +12,4 @@ describe('File', () => {
     const actual = new File('foobar.js', Buffer.from('string-content'));
     expect(actual.textContent).deep.eq('string-content');
   });
-
-  it('should be serializable', () => {
-    const expected = new File('foo', Buffer.from('bar'));
-    const serialized = serialize(expected);
-    const actual = deserialize(serialized, [File]);
-    expect(actual).deep.eq(expected);
-    expect(actual).instanceOf(File);
-  });
-
-  /**
-   * @see https://github.com/stryker-mutator/stryker-js/issues/2025
-   */
-  it('should customize serialization to allow different instances of the class file to be compatible', () => {
-    expect(new File('foo', Buffer.from('bar')).surrialize()).eq('new File("foo", Buffer.from("bar", "binary"))');
-  });
 });

packages/core/package.json

@@ -79,7 +79,6 @@
     "rimraf": "~3.0.0",
     "rxjs": "~6.6.0",
     "source-map": "~0.7.3",
-    "surrial": "~2.0.2",
     "tree-kill": "~1.2.2",
     "tslib": "~2.2.0",
     "typed-inject": "~3.0.0",

packages/core/src/child-proxy/child-process-proxy-worker.ts

@@ -1,6 +1,5 @@
 import path from 'path';
 
-import { File } from '@stryker-mutator/api/core';
 import { errorToString } from '@stryker-mutator/util';
 import { getLogger, Logger } from 'log4js';
 
@@ -23,12 +22,12 @@ export class ChildProcessProxyWorker {
 
   private send(value: ParentMessage) {
     if (process.send) {
-      const str = serialize(value, [File]);
+      const str = serialize(value);
       process.send(str);
     }
   }
   private handleMessage(serializedMessage: string) {
-    const message = deserialize<WorkerMessage>(serializedMessage, [File]);
+    const message = deserialize<WorkerMessage>(serializedMessage);
     switch (message.kind) {
       case WorkerMessageKind.Init:
         this.handleInit(message);

packages/core/src/child-proxy/child-process-proxy.ts

@@ -1,7 +1,7 @@
 import childProcess from 'child_process';
 import os from 'os';
 
-import { File, StrykerOptions } from '@stryker-mutator/api/core';
+import { StrykerOptions } from '@stryker-mutator/api/core';
 import { PluginContext } from '@stryker-mutator/api/plugin';
 import { isErrnoException, Task, ExpirableTask } from '@stryker-mutator/util';
 import { getLogger } from 'log4js';
@@ -87,7 +87,7 @@ export class ChildProcessProxy<T> implements Disposable {
   }
 
   private send(message: WorkerMessage) {
-    this.worker.send(serialize(message, [File]));
+    this.worker.send(serialize(message));
   }
 
   private initProxy(): Promisified<T> {
@@ -127,7 +127,7 @@ export class ChildProcessProxy<T> implements Disposable {
 
   private listenForMessages() {
     this.worker.on('message', (serializedMessage: string) => {
-      const message: ParentMessage = deserialize(serializedMessage, [File]);
+      const message = deserialize<ParentMessage>(serializedMessage);
       switch (message.kind) {
         case ParentMessageKind.Initialized:
           this.initTask.resolve(undefined);

packages/core/src/config/index.ts

@@ -1,4 +1,5 @@
 export * from './read-config';
 export * from './options-validator';
+export * from './options-marker';
 export * from './build-schema-with-plugin-contributions';
 export * from './file-matcher';

packages/core/src/config/options-marker.ts

@@ -0,0 +1,65 @@
+import type { JSONSchema7 } from 'json-schema';
+import { tokens } from 'typed-inject';
+import { StrykerOptions } from '@stryker-mutator/api/core';
+import { commonTokens } from '@stryker-mutator/api/plugin';
+import { Logger } from '@stryker-mutator/api/logging';
+import { PropertyPathBuilder, findUnserializables } from '@stryker-mutator/util';
+
+import { coreTokens } from '../di';
+import { isWarningEnabled } from '../utils/object-utils';
+
+markOptions.inject = tokens(commonTokens.options, coreTokens.validationSchema, commonTokens.logger);
+
+/**
+ * Performs additional validation on the Stryker options to mark unusual behavior with a warning.
+ * Namely when a value isn't serializable or when unknown options are passed in.
+ */
+export function markOptions(options: StrykerOptions, schema: JSONSchema7, log: Logger): StrykerOptions {
+  markUnknownOptions(options, schema, log);
+  markUnserializableOptions(options, log);
+  return options;
+}
+
+function markUnknownOptions(options: StrykerOptions, schema: JSONSchema7, log: Logger) {
+  const OPTIONS_ADDED_BY_STRYKER = ['set', 'configFile', '$schema'];
+
+  if (isWarningEnabled('unknownOptions', options.warnings)) {
+    const schemaKeys = Object.keys(schema.properties!);
+    const unknownPropertyNames = Object.keys(options)
+      .filter((key) => !key.endsWith('_comment'))
+      .filter((key) => !OPTIONS_ADDED_BY_STRYKER.includes(key))
+      .filter((key) => !schemaKeys.includes(key));
+
+    if (unknownPropertyNames.length) {
+      unknownPropertyNames.forEach((unknownPropertyName) => {
+        log.warn(`Unknown stryker config option "${unknownPropertyName}".`);
+      });
+
+      const p = PropertyPathBuilder.create<StrykerOptions>().prop('warnings').prop('unknownOptions').build();
+
+      log.warn(`Possible causes:
+   * Is it a typo on your end?
+   * Did you only write this property as a comment? If so, please postfix it with "_comment".
+   * You might be missing a plugin that is supposed to use it. Stryker loaded plugins from: ${JSON.stringify(options.plugins)}
+   * The plugin that is using it did not contribute explicit validation. 
+   (disable "${p}" to ignore this warning)`);
+    }
+  }
+}
+
+function markUnserializableOptions(options: StrykerOptions, log: Logger) {
+  if (isWarningEnabled('unserializableOptions', options.warnings)) {
+    const unserializables = findUnserializables(options);
+    if (unserializables) {
+      unserializables.forEach(({ reason, path }) =>
+        log.warn(
+          `Config option "${path.join(
+            '.'
+          )}" is not (fully) serializable. ${reason}. Any test runner or checker worker processes might not receive this value as intended.`
+        )
+      );
+      const p = PropertyPathBuilder.create<StrykerOptions>().prop('warnings').prop('unserializableOptions').build();
+      log.warn(`(disable ${p} to ignore this warning)`);
+    }
+  }
+}