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

Adding serializer to remove internal angular attributes from the snapshots #97

Merged
merged 10 commits into from
Apr 28, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ jobs:
steps:
- attach_workspace:
at: .
- restore_cache:
keys:
- yarn-cache-example-{{ .Branch }}-{{ checksum "yarn.lock" }}
- yarn-cache-example-{{ .Branch }}-
#- restore_cache:
# keys:
# - yarn-cache-example-{{ .Branch }}-{{ checksum "yarn.lock" }}
# - yarn-cache-example-{{ .Branch }}-
- run:
name: Install Example Dependencies
command: |
cd example
rm -rf node_modules/jest-preset-angular
yarn install --frozen-lockfile
- save_cache:
key: yarn-cache-example-{{ .Branch }}-{{ checksum "yarn.lock" }}
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
node_modules
InlineHtmlStripStylesTransformer.js
*.log
.idea
34 changes: 34 additions & 0 deletions AngularNoNgAttributesSnapshotSerializer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';

const jestDOMElementSerializer = require('pretty-format').plugins.DOMElement;

const attributesToRemovePatterns = ['ng-reflect', '_nghost', '_ngcontent', 'ng-version'];

const serialize = (node, ...rest) => {
const nodeCopy = node.cloneNode(true);
Object.values(nodeCopy.attributes)
.map(attribute => attribute.name)
.filter(attributeName =>
attributesToRemovePatterns
.some(removeAttributePattern => attributeName.startsWith(removeAttributePattern))
wtho marked this conversation as resolved.
Show resolved Hide resolved
)
.forEach(attributeNameToRemove => nodeCopy.attributes.removeNamedItem(attributeNameToRemove));

return jestDOMElementSerializer.serialize(nodeCopy, ...rest);
};

const serializeTestFn = (val) => {
return val.attributes !== undefined && Object.values(val.attributes)
.map(attribute => attribute.name)
wtho marked this conversation as resolved.
Show resolved Hide resolved
.some(attributeName =>
attributesToRemovePatterns
.some(remotePattern => attributeName.startsWith(remotePattern))
wtho marked this conversation as resolved.
Show resolved Hide resolved
)
};
const test = val =>
jestDOMElementSerializer.test(val) && serializeTestFn(val);

module.exports = {
serialize: serialize,
test: test
};
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## Changelog (master)

* Added `AngularNoNgAttributesSnapshotSerializer`. Using this serializer makes snapshots clearer and more human-readable. You have to apply this serializer manually by redefining `snapshotSerializers` `jest` option.

### v7.0.0

#### Features
Expand Down
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# jest-preset-angular
AngularSnapshotSerializer# jest-preset-angular
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Done.


[![CircleCI Build Status](https://circleci.com/gh/thymikee/jest-preset-angular.svg?style=shield&circle-token=:circle-token)](https://circleci.com/gh/thymikee/jest-preset-angular)
[![NPM version](https://img.shields.io/npm/v/jest-preset-angular.svg)](https://www.npmjs.com/package/jest-preset-angular) [![Greenkeeper badge](https://badges.greenkeeper.io/thymikee/jest-preset-angular.svg)](https://greenkeeper.io/)
Expand Down Expand Up @@ -69,6 +69,7 @@ module.exports = {
},
transformIgnorePatterns: ['node_modules/(?!@ngrx)'],
snapshotSerializers: [
'jest-preset-angular/AngularNoNgAttributesSnapshotSerializer.js',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not included by default yet, please revert

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

'jest-preset-angular/AngularSnapshotSerializer.js',
'jest-preset-angular/HTMLCommentSerializer.js',
],
Expand All @@ -84,7 +85,8 @@ module.exports = {
- `"moduleFileExtensions"` – our modules are TypeScript and JavaScript files
- `"moduleNameMapper"` – if you're using absolute imports here's how to tell Jest where to look for them; uses regex
- `"setupFilesAfterEnv"` – this is the heart of our config, in this file we'll setup and patch environment within tests are running
- `"transformIgnorePatterns"` – unfortunately some modules (like @ngrx ) are released as TypeScript files, not pure JavaScript; in such cases we cannot ignore them (all node_modules are ignored by default), so they can be transformed through TS compiler like any other module in our project.
- `"transformIgnorePatterns"` – unfortunately some modules (like @ngrx) are released as TypeScript files, not pure JavaScript; in such cases we cannot ignore them (all node_modules are ignored by default), so they can be transformed through TS compiler like any other module in our project.
- `"snapshotSerializers"` - array of serializers which will be applied to snapshot the code. Note: by default angular adds some angular-specific attributes to the code (like `ng-reflect-*`, `ng-version="*"`, `_ngcontent-c*` etc). This package provides serializer to remove such attributes. This makes snapshots cleaner and more human-readable. To remove such specific attributes use `AngularNoNgAttributesSnapshotSerializer` serializer. You need to add `AngularNoNgAttributesSnapshotSerializer` serializer manually (like in `example` application in `package.json`).
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a link to the exact lines in package.json?


## [AST Transformer](https://github.com/thymikee/jest-preset-angular/blob/master/src/InlineHtmlStripStylesTransformer.ts)

Expand Down
7 changes: 6 additions & 1 deletion example/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"build": "ng build",
"test": "jest",
"test:watch": "jest --watch",
"test:ci": "jest --runInBand",
"test:ci": "jest --runInBand --no-cache",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why no-cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wtho looks like it was your change... do we still need it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be removed!

"test:coverage": "jest --coverage",
"lint": "ng lint",
"e2e": "ng e2e"
Expand Down Expand Up @@ -44,6 +44,11 @@
},
"jest": {
"preset": "jest-preset-angular",
"snapshotSerializers": [
"jest-preset-angular/AngularNoNgAttributesSnapshotSerializer.js",
"jest-preset-angular/AngularSnapshotSerializer.js",
"jest-preset-angular/HTMLCommentSerializer.js"
],
"moduleNameMapper": {
"\\.(jpg|jpeg|png)$": "<rootDir>/__mocks__/image.js",
"^@lib/(.*)$": "<rootDir>/src/lib/$1"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ABitMoreComplexComponent snapshot test 1`] = `
<app-a-bit-more-complex
anotherVar="false"
someVar={[Function Boolean]}
>
<div
class="some-wrapper-class"
>
diary works!

<div
class="inner-class"
>

<div>
another case
wtho marked this conversation as resolved.
Show resolved Hide resolved
</div>
<span>
some html
</span>
</div>
<div>
one more case case
</div>
<div>
<app-child-component
someinput="someVar"
>
stubbedBody
</app-child-component>
</div>
</div>
</app-a-bit-more-complex>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/* tslint:disable:no-unused-variable */
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

future notice: we should move to eslint with TS support

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah!
But I would suggest to wait for Angular to make the official move to eslint, as this is still an Angular app example (might come in v8?).

Also see the official tslint -> eslint issue

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

import { ABitMoreComplexComponent } from './a-bit-more-complex.component';
import { ChildComponent } from './child.component';

describe('ABitMoreComplexComponent', () => {
let component: ABitMoreComplexComponent;
let fixture: ComponentFixture<ABitMoreComplexComponent>;

beforeEach(
async(() => {
TestBed.configureTestingModule({
declarations: [ABitMoreComplexComponent, ChildComponent],
})
.overrideComponent(ChildComponent, {set: {template: 'stubbedBody'}})
.compileComponents();
}),
);

beforeEach(() => {
fixture = TestBed.createComponent(ABitMoreComplexComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});

it('should create', () => {
expect(component).toBeTruthy();
});

it('snapshot test', () => {
expect(fixture).toMatchSnapshot();
});
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The component, test file and the test suite should not just be named "a bit more complex component", we have also other tests than the serializers, so it should definitely explain, that it is a serializer-test.

You can name it something like "complex templating component" or name what is complex about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this code was wrote very long time ago, but still... It was "copy-pasted" from other specs in example project.

example/src/app/calc/calc.component.spec.ts:

  it('should snap', () => {
    expect(fixture).toMatchSnapshot();
  });

example/src/app/simple/simple.component.spec.ts:

  it('snapshots', () => {
    expect(fixture).toMatchSnapshot();
  })

And stuff...

I understand your point. Is it critical? If yes, other code have to be refactored too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Still I think "SimpleComponent" alone is clear what to expect in the component. "ABitMoreComplexComponent" will always only stand in comparison to "SimpleComponent", which is not as good.

I would recommend e. g. "MediumComplexComponent"

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Component } from '@angular/core';

@Component({
selector: 'app-a-bit-more-complex',
wtho marked this conversation as resolved.
Show resolved Hide resolved
template: `
<div class="some-wrapper-class">
diary works!
<div *ngIf="someVar" class="inner-class">
<div *ngIf="anotherVar" class="inner-hidden"></div>
<div>another case</div>
<span>some html</span>
</div>
<div>one more case case</div>
<div><app-child-component someInput="someVar"></app-child-component></div>
</div>
`
})
export class ABitMoreComplexComponent {
someVar = true;
anotherVar = false;
}
20 changes: 20 additions & 0 deletions example/src/app/a-bit-more-complex-component/child.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Component, Input } from '@angular/core';

@Component({
selector: 'app-child-component',
template: `
<div class="child">
{{ someInput }}
<div>
rly
<p>complex</p>
<div>component
<div>oh my god!</div>
</div>
</div>
</div>
`
})
export class ChildComponent {
@Input() someInput = null;
}
Empty file removed example/src/app/app.component.css
Empty file.
9 changes: 3 additions & 6 deletions example/src/app/app.component.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
import {Component, HostBinding, OnInit} from '@angular/core';
import {Component, HostBinding} from '@angular/core';
import {trigger, transition, style, animate} from '@angular/animations';

@Component({
selector: 'app-root',
animations: [slideToLeft()],
templateUrl: './app.component.html',
styleUrls: ['./app.component.css'],
templateUrl: './app.component.html'
})
export class AppComponent implements OnInit{
export class AppComponent {
@HostBinding('@routerTransition')
title = 'app works!';
hasClass = true;
variableWithPrecedingDolar = 1234;

ngOnInit() {}
}

export function slideToLeft() {
Expand Down
6 changes: 5 additions & 1 deletion example/src/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ import { CalcComponent } from './calc/calc.component';
import { SimpleComponent } from './simple/simple.component';
import { OnPushComponent } from './on-push/on-push.component';
import { HeroesComponent } from './heroes/heroes.component';
import { SimpleWithStylesComponent } from './simple-with-styles/simple-with-styles.component';
import { ChildComponent } from './a-bit-more-complex-component/child.component';

@NgModule({
declarations: [
AppComponent,
CalcComponent,
SimpleComponent,
OnPushComponent,
HeroesComponent
HeroesComponent,
SimpleWithStylesComponent,
ChildComponent
],
imports: [
BrowserModule,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ exports[`CalcComponent should snap 1`] = `
>
<p
class="a-default-class"
ng-reflect-klass="a-default-class"
ng-reflect-ng-class="[object Object]"
>
calc works! 1337 another text node test-image-stub
</p>
Expand Down
Empty file.
3 changes: 1 addition & 2 deletions example/src/app/calc/calc.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ const image = require('assets/its_something.png');
another text node
{{image}}
</p>
`,
styleUrls: ['./calc.component.css']
`
})
export class CalcComponent implements OnInit {
@Input() hasAClass = false;
Expand Down
1 change: 0 additions & 1 deletion example/src/app/heroes/heroes.component.css

This file was deleted.

3 changes: 1 addition & 2 deletions example/src/app/heroes/heroes.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import { HeroService } from '../service/hero.service';

@Component({
selector: 'app-heroes',
templateUrl: './heroes.component.html',
styleUrls: ['./heroes.component.css']
templateUrl: './heroes.component.html'
})
export class HeroesComponent implements OnInit {
heroes: Hero[];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`NgReflectAsTextComponent snapshots 1`] = `
<app-ng-reflect-as-text>
<p
data-some-attr="ng-reflect-as-attr-arg works"
>
ng-reflect-as-text works!

</p>
</app-ng-reflect-as-text>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<p data-some-attr="ng-reflect-as-attr-arg works">
ng-reflect-as-text works!
wtho marked this conversation as resolved.
Show resolved Hide resolved
</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { NgReflectAsTextComponent } from './ng-reflect-as-text.component';

describe('NgReflectAsTextComponent', () => {
let component: NgReflectAsTextComponent;
let fixture: ComponentFixture<NgReflectAsTextComponent>;

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [ NgReflectAsTextComponent ]
})
.compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(NgReflectAsTextComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});

it('snapshots', () => {
expect(fixture).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { Component } from '@angular/core';

@Component({
selector: 'app-ng-reflect-as-text',
templateUrl: './ng-reflect-as-text.component.html'
})
export class NgReflectAsTextComponent {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`SimpleWithStylesComponent should generate snapshot without nghost/ngcontent 1`] = `
<app-simple-with-styles>
<p>
simple-with-styles works!

</p><div
class="some-class"
>
<app-child-component>
<div
class="child"
>

<div>
rly
<p>
complex
</p>
<div>
component
<div>
oh my god!
</div>
</div>
</div>
</div>
</app-child-component>
</div>
</app-simple-with-styles>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<p>
simple-with-styles works!
</p>
<div class="some-class">
<app-child-component></app-child-component>
</div>
Loading