Stryker v4 Instrumentor breaks enzyme snapshots

[Garethp]

Summary
When using Stryker v4, my snapshots (using enzyme) break because where in the main run it recognizes the component name, in the mutant run it just calls it Component.

Example:

const ErrorMessage = ({ message, id }: Props) => (
  <React.Fragment>
    {message.length > 0 && (
      <span id={id} className="error-message">
        {message}
        <span className="hidden">Error: {message}</span>
      </span>
    )}
  </React.Fragment>
);

export default ErrorMessage;

interface IFormLabelProps {
  label: string;
  forId: string;
  errorMessage?: string;
}

const FormLabel: FunctionComponent<IFormLabelProps> = ({
  label,
  forId,
  errorMessage
}) => {
  return (
    <>
      <label className="label" htmlFor={forId}>
        {label}
      </label>
      {!!errorMessage?.length && (
        <ErrorMessage id={`${forId}-error`} message={errorMessage} />
      )}
    </>
  );
};

it("should render with error message", () => {
  const formLabel = shallow(
    <FormLabel
      label="test label"
      forId="testId"
      errorMessage="test error message"
    />
  );
  expect(formLabel).toMatchSnapshot();
});

In the main run it will return the snapshot

exports[`should render with error message 1`] = `
<Fragment>
  <label
    className="label"
    htmlFor="testId"
  >
    test label
  </label>
  <ErrorMessage
    id="testId-error"
    message="test error message"
  />
</Fragment>
`;

While in the mutant it'll render

exports[`should render with error message 1`] = `
<Fragment>
  <label
    className="label"
    htmlFor="testId"
  >
    test label
  </label>
  <Component
    id="testId-error"
    message="test error message"
  />
</Fragment>
`;

The difference being that ErrorMessage has been replaced with Component. This is because the instrumentor replaced the main definition of ErrorMessage with a ternary, and it appears that enzyme (or react, not sure) is looking for the variable name of the original variable that the component is assigned to.

If you replaced ErrorMessage with

const ErrorMessage = true ? ({ message, id }: Props) => (
  <React.Fragment>
    {message.length > 0 && (
      <span id={id} className="error-message">
        {message}
        <span className="hidden">Error: {message}</span>
      </span>
    )}
  </React.Fragment>
) : () => undefined;

Then the snapshot would also fail in the same way it does during mutation tests.

One possible fix is to temporarily assign the component to a variable with the original name inside the ternary. That way the component maintains its original variable name. Here's an example below:

let ErrorMessage;

const ErrorMessageInstrumented = true
  ? (ErrorMessage = ({ message, id }: Props) => (
      <React.Fragment>
        {message.length > 0 && (
          <span id={id} className="error-message">
            {message}
            <span className="hidden">Error: {message}</span>
          </span>
        )}
      </React.Fragment>
    ))
  : () => undefined;

export default ErrorMessageInstrumented;

[nicojs]

Hi @Garethp thanks a lot for opening this issue. Are you still planning to add a e2e test with enzyme snapshots? It's optional of course, but it would help us a lot with solving this issue. It's also fine if you just add the files a new e2e/test directory (for example e2e/test/jest-with-enzyme-snapshots). If you need any help, don't hesitate to react here, or hit us up on slack

[Garethp]

Yes I am, I just didn't find the time this weekend. I should have a PR in the next day or so

[nicojs]

The problem is a little bit more broad even, as you can see here:

$ node -p "const Foo = class { }; Foo.name"
Foo
$ node -p "const Foo = true? class { }: ''; Foo.name"

[nicojs]

I've been thinking about this and I think I know of a solution that works for most use cases.

I want to instrument these:

const foo = function () { }
const bar = () => {}
const Baz = class { }

Like this:

const foo = true? function foo () { }: ...;
const bar = true? (() => { const bar = () =>{}; return bar } {})() : ...;
const Baz = true? class Baz { } : ...;

What do you think of this?

I don't like your approach @Garethp, because it changes the functionality of the code too much and will be difficult to maintain.

[Garethp]

That looks like it works quite well. I'll test it on my work project when it's been pushed up

[nicojs]

I've just released 4.0.0-beta.3 with this fix. Feel free to give it another go @Garethp 🌹

[Garethp]

This is strange, but there are some cases where you might not want to be doing this? Take the following case:

export const SomeItem = () => <div></div>;

For some reason in non-mutated code this gets exported without a name. In the mutated code it now does have a name? I didn't even notice this before.

Also, it looks like there's a bug where having a string that starts with https:// as a return seems to break the instrumentor and it spits out an unterminated string. I'll try to get that as a e2e test in the next couple of days