Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

snapshot feature request: Allow option to remove trailing whitespace #287

Closed
abierbaum opened this issue Jun 19, 2019 · 15 comments · Fixed by #961
Closed

snapshot feature request: Allow option to remove trailing whitespace #287

abierbaum opened this issue Jun 19, 2019 · 15 comments · Fixed by #961
Labels

Comments

@abierbaum
Copy link

This project is working great for our team. We especially like the way we can do inline snapshots for angular components in our test suite. One problem we have run into though is that when we create an inline snapshot and the snapshot text has trailing whitespace, our text editors (vscode, vim, etc) will automatically strip the whitespace and thus make the snapshot fail to compare equal the next time the test is run.

Would it be possible to add an option to the AngularSnapshotSerializer such that the print method automatically strips trailing whitespace upon snapshot creation? I would be willing to work up a PR if I knew this was of interest to the project and if someone could point me at a recommended way to allow this override / option to be added. I don't see a clean way to pass in options to snapshot serializers.

If there is some other way to handle this I would be very interested as well.

@wtho
Copy link
Collaborator

wtho commented Jun 19, 2019

What do you mean with "inline snapshots"? Like snapshots created by the VSCode jest extension? How does vim fiddle with the snapshots? Formatters should not handle snapshot files.

Anyway, did you play with the preserveWhitespaces option? I never had the issue and used it, but this might solve it already.

You can also just copy
node_modules/jest-preset-angular/AngularNoNgAttributesSnapshotSerializer.js (which replaces the standard html serializer)
or
node_modules/jest-preset-angular/AngularSnapshotSerializer.js (which serializes Angular fixtures/DebugElements)
to your project, extend it with the trimming and set it as snapshotSerializers in your jest config.

As far as I see there is no custom option you can set and pass to a serializer, so a custom implementation is probably the easiest way for now.

In case this is a problem for others as well, we are always open to PRs.

@thymikee
Copy link
Owner

@wtho
Copy link
Collaborator

wtho commented Jun 21, 2019

Ah, now I get it! I never used inline snapshots, good to know!

So I assume it is the code formatters that touch the snapshots if there are trailing whitespaces. Maybe you can disable it for *.spec.ts?

Anyway I looked at the AngularSnapshotSerializer and it does always end with a >, so there should not be trailing whitespaces at all.

return (
'<' +
componentName +
componentAttrs +
(componentAttrs.length ? '\n' : '') +
'>\n' +
indent(nodes) +
'\n</' +
componentName +
'>'
);

To make sure you use the AngularSnapshotSerializer.js, always pass the fixture (returned from testBed.createComponent(...)) to the expect.

@abierbaum
Copy link
Author

@wtho Here is an example. If there is a working live code template on stackblitz you can point me at, I could make it live there if that helps.

The key issues is the places where there end up being empty lines in the snapshot. The snapshot generation code from AngularSnapshotSerializer creates those with whitespace indentation. All of our developers have their editors set to strip training whitespace for all code, so when the files get saved, those lines get reformatted and no longer match the snapshot. I tried finding a way to configure the editors to ignore the files, but was unable to find one and it just leads to another issue where the rest of the code in that file would not get formatted correctly. I think what is really a better solution would be for the snapshot generation not to output empty lines.

As an aside, from this example I don't quite understand why the "{[Function Boolean]}" and "{[Function String]}" are getting in there, but that is a different issue.

Note: this is with jest-preset-angular version 7.1.0.

import {Component, Input} from '@angular/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';

/**
 * Example test component.
 */
@Component({
   selector: 'tc-jest-inline-test1',
   template: `
      <div>Line 1</div>
      <div>
         <div *ngIf="condition1">
            {{ value1 }}
         </div>
         <span *ngIf="condition2">
            {{ value2 }}
         </span>
      </div>
   `,
})
export class JestInlineSnapBugComponent {
   @Input() value1: string = 'val1';
   @Input() value2: string = 'val2';

   condition1: boolean = true;
   condition2: boolean = false;
}

describe('Jest Snapshot Bug', () => {
   let comp: JestInlineSnapBugComponent;
   let fixture: ComponentFixture<JestInlineSnapBugComponent>;

   beforeEach(() => {
      TestBed.configureCompiler({preserveWhitespaces: false} as any);
      TestBed.configureTestingModule({
         declarations: [JestInlineSnapBugComponent],
      });
      fixture = TestBed.createComponent(JestInlineSnapBugComponent);
      comp = fixture.componentInstance;
      fixture.detectChanges();
   });

   it('should allow snapshots', () => {
      expect(fixture).toMatchInlineSnapshot(`
         <tc-jest-inline-test1
           condition1={[Function Boolean]}
           condition2="false"
           value1={[Function String]}
           value2={[Function String]}
         >
           <div>
             Line 1
           </div><div>
             
             <div>
                val1 
             </div>
             
           </div>
         </tc-jest-inline-test1>
      `);

      comp.condition2 = true;
      fixture.detectChanges();
      expect(fixture).toMatchInlineSnapshot(`
         <tc-jest-inline-test1
           condition1={[Function Boolean]}
           condition2={[Function Boolean]}
           value1={[Function String]}
           value2={[Function String]}
         >
           <div>
             Line 1
           </div><div>
             
             <div>
                val1 
             </div>
             
             <span>
                val2 
             </span>
           </div>
         </tc-jest-inline-test1>
      `);

      comp.condition1 = false;
      comp.condition2 = false;
      expect(fixture).toMatchInlineSnapshot(`
         <tc-jest-inline-test1
           condition1="false"
           condition2="false"
           value1={[Function String]}
           value2={[Function String]}
         >
           <div>
             Line 1
           </div><div>
             
             <div>
                val1 
             </div>
             
             <span>
                val2 
             </span>
           </div>
         </tc-jest-inline-test1>
      `);
   });
});

@abierbaum
Copy link
Author

@wtho Any ideas on the example I provided. I am guessing I am doing something wrong but I still can't see what.

@wtho
Copy link
Collaborator

wtho commented Jun 30, 2019

So I looked at it, and jest/this preset looks like it's acting fine.

First I added your tests to a project (in my case the test project of this repo) without inline snapshots and then I let jest generate the snapshots.

Here are the changes of the snapshot generation.

Also have a look at the complete generated file.

If I run my formatter (in my case prettier), it does not change anything, everything is formatted already. It does not touch the template strings, as indentation inside strings is changing the string itself, and changing a value by adding spaces can manipulate app behavior.

I don't know how your indented snapshots were created, but it seems like your formatter removed/added spaces from/to them. I think it does not know JavaScript/TypeScript template literals and that they can be multiline. If this is the case I highly recommend you to use a sane, JS-aware formatter.

If this is no option for you, you can also use snapshots in dedicated .snap-files.


{[Function String]} depicts a reference of a function. In the case of @Input() value1: string = "val1"; Angular probably creates a reactive getter function for value1, in this case to the value val1. Without invoking it, the JS runtime does also know that this function returns a JS String. Jest just wants to inform you, what is referenced here, and does this by rendering {[Function String]}.

@abierbaum
Copy link
Author

For anyone else that runs into this issue, I created a very simple snapshot serializer that wraps the AngularSnapshotSerializer and strips trailing whitespace. I know that its technically removes some of the whitespace from the template output that could count, but for our case it works very well.

Hoping it may be useful for someone else in the future.

/**
 * Custom serializer that removes whitespace at end of lines.
 *
 * This helps when using toMatchInlineSnapshot in editors that automatically
 * strip trailing newline characters on save.  (ex: vscode)
 *
 * see: https://github.com/thymikee/jest-preset-angular/issues/287
 */
const ng_serializer = require("jest-preset-angular/AngularSnapshotSerializer.js");

const printOverride = (val, print, indent, opts, colors) => {
   return ng_serializer.print(val, print, indent, opts, colors).replace(/[ \t]*$/gm, '');
};

module.exports = {
   print: printOverride,
   test: ng_serializer.test,
};

In the meantime I think we can probably close this issue unless anyone thinks this should be added to the default serializer.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Sep 12, 2019

In the meantime I think we can probably close this issue unless anyone thinks this should be added to the default serializer.

@abierbaum is it possible to create a serializer from your solution which can be imported optionally by users, similar to the current serializers provided by this preset ?

@ahnpnl ahnpnl added the 🚀 Feature Request new suggested feature label Sep 12, 2019
@abierbaum
Copy link
Author

@ahnpnl Yes. That is what I do with the serializer above. I just save this code into a file in my repository and reference it as the custom serializer.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Sep 12, 2019

I think this one is a new one. Would be great to have a PR from your side to add this to this preset 😀

@intellix
Copy link
Contributor

I think one of the most disruptive parts of snapshots for us is spurious new lines declaring the snapshot as failing.

I believe comments/ng-containers/templates are being replaced by empty new lines. You can update a library and have snapshots throughout fail due to a random new line (which are irrelevant due to preserveWhitespace: false being default:

It happens like crazy when using ngx-formly for example:
Screenshot 2019-10-15 at 17 44 12

@wtho
Copy link
Collaborator

wtho commented Oct 15, 2019

Internally we use pretty-format's DOMElement serializer, which lives inside facebook/jest.
I would suggest to open a PR at facebook/jest or to open a PR here with a modified, rewritten dom element serializer.

Another solution is to adjust the output of the serializer as done by @abierbaum. PRs are always welcome!

@intellix
Copy link
Contributor

Tried the solution from @abierbaum but still had a lot of empty lines. It removed the tabs which was an improvement, but this one completely removes empty lines:

module.exports = {
  print: (val: any, print: any, indent: any, opts: any, colors: any) => {
    return serializer.print(val, print, indent, opts, colors).replace(/\n^\s*\n/gm, '\n');
  },
  test: serializer.test,
};

@Stephanemw
Copy link

Stephanemw commented Jan 27, 2021

Thanks @abierbaum @intellix ! This sorted out my problem with different empty lines appearing between local and ci test runs.

@wtho
Copy link
Collaborator

wtho commented Jan 28, 2021

Feel free to open a PR to make this solution available to a wider user base!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants