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

Different snapshot with --coverage #220

Closed
Ludevik opened this issue Jan 14, 2019 · 11 comments
Closed

Different snapshot with --coverage #220

Ludevik opened this issue Jan 14, 2019 · 11 comments

Comments

@Ludevik
Copy link

Ludevik commented Jan 14, 2019

I am currently using [email protected] and our CI builds started to failing due to different snapshot when using jest with --coverage:

-                   ng-reflect-product-filter="function (product) { return pr"
+                   ng-reflect-product-filter="function (product) {
+            "

This started to happend when I upgraded to version 7 which uses ts-jest. Not sure who is responsible for serializing components. There is similar issue, but I don't see anything related to istanbul in my snapshot.

@wtho
Copy link
Collaborator

wtho commented Jan 16, 2019

It is probably on our side, I guess it results from the combination of the Serializer and the modified html stringification of ts-jest 23.10.

Can you post your (minimal) component and your test? This would simplify the process for us to look into it.

@Ludevik
Copy link
Author

Ludevik commented Jan 18, 2019

I finally managed to reproduce this.

export class Predicates {
    static readonly doSomething = (): boolean => true;
}
import { Component } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { Predicates } from './predicates';

@Component({
	selector: 'test',
	template: '{{func}}'
})
class TestComponent {
	func: () => boolean;
}

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

	beforeEach(() => {
		TestBed.configureTestingModule({
			declarations: [TestComponent]
		});
		fixture = TestBed.createComponent(TestComponent);
		component = fixture.componentInstance;
		component.func = Predicates.doSomething;
		fixture.detectChanges();
	});

	it('should render test component', () => {
		expect(fixture).toMatchSnapshot();
	});
});
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`test should render test component 1`] = `
<test
  func={[Function Function]}
>
  function () { return true; }
</test>
`;
● test › should render test component

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "test should render test component 1".

    - Snapshot
    + Received

      <test
        func={[Function Function]}
      >
    -   function () { return true; }
    +   function () {
    +           /* istanbul ignore next */cov_2d033xhqn0.f[2]++;
    +           cov_2d033xhqn0.s[3]++;
    +           return true;
    +       }
      </test>

      26 |
      27 |      it('should render test component', () => {
    > 28 |              expect(fixture).toMatchSnapshot();
         |                              ^
      29 |      });
      30 | });
      31 |

It happens when the template renders some function, e.g. when used as an input to some child component. The function itself must be in different file, otherwise there is no difference.

Now when i prepared the simple test case i can see that in it is the same problem with Istanbul as here.

@wtho
Copy link
Collaborator

wtho commented Jan 20, 2019

Thank you for the reproduction!

It seems that if runnable code is imported and injected into templates in this way, we have to filter out the istanbul coverage tags manually. Which might become a mess.
Maybe this is already done in some serializer in other framework, but I personally have never dived that much into Istanbul.

cc @thymikee @ahnpnl

@thymikee
Copy link
Owner

Jest should replace functions with a [Function] or some similar tag. But in your example the function that is being snapshot is getting stringified before running through our html serializer.

I'd say this is not something you'd like to snapshot, and I don't think there's really anything from this preset nor Jest's side that we can do that makes sense. Feel free to write a small serializer that replaces istanbul comments with some token or anything that doesn't change.

Closing, I hope you understand.

@Ludevik
Copy link
Author

Ludevik commented Jan 20, 2019

@thymikee this was just simplest example i could think of. In my real setup i don't serialize the function to html directly, but it is an @Input to other component and serialization is just side effect. It looks something like this:

@Component({
	selector: 'test',
	template: '<child-component [predicate]="predicate"></child-component>'
})
class TestComponent {
    get predicate(): () => boolean {
        return Predicates.doSomething;
    }
}

Current behaviour doesn't allow to snapshot test a component which uses some other component with function @Inputs when used together with coverage. There are currently component libraries out there which use functions as inputs.

I'll try to write the serializer and see how far i can get ;)

@Ludevik
Copy link
Author

Ludevik commented May 17, 2019

So i was forced to finally write the serializer as our CI rendered functions differently than local machine and not only for coverage. Whole problem is that Angular serializes function inputs in such way that it takes the implementations first N characters and writes them into the DOM. So it renders something like this:

<component ng-reflect-some-input="function (...">

I wrote a serializer that checks whether such attribute exists and just replaces it with [{Function Function}] and leaves the serialization on presets serializer. Feels a bit hackish but it gets the job done ;)

'use strict';

//this serializer replaces function implementations that are rendered like: `function (` with `{[Function Function]}`

const getInvalidAttributes = val =>
	val.getAttributeNames().filter(attr => val.getAttribute(attr).startsWith('function ('));

const print = (val, print) => {
	getInvalidAttributes(val).forEach(attr => val.setAttribute(attr, '{[Function Function]}'));
	return print(val);
};

const test = val => {
	return (
		val !== undefined &&
		val !== null &&
		typeof val.getAttributeNames === 'function' &&
		getInvalidAttributes(val).length
	);
};

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

@ahnpnl
Copy link
Collaborator

ahnpnl commented May 17, 2019

Recently we merged a similar serializer in #97. Maybe you can try master branch in your local to see if it improves the situation ? Perhaps @thymikee can release a alpha or beta version to test the new serializer.

@thymikee
Copy link
Owner

@ahnpnl it's released in 7.1.0, needs to be added manually though (there are docs on that)

@ahnpnl
Copy link
Collaborator

ahnpnl commented May 17, 2019

@Ludevik I updated changelog for 7.1.0, you can try the new serializer 👍

@Ludevik
Copy link
Author

Ludevik commented May 17, 2019

Cool, if only i've reread the readme after release i could spare myself from reinventing the wheel, but at least i got some insights how serializers work in jest :)
I tested the AngularNoNgAttributesSnapshotSerializer and it helps me. ng-reflect-* is not interesting from my perspective, so it does good job ;)
There is one issue though, the new serializer is not part of the npm package, i had to manually copy it from github into my project to test it.

@thymikee
Copy link
Owner

@Ludevik always learning, that's the attitude! I could've at least pushed the tag but forgot about it as I made a release during my time off 😅

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

No branches or pull requests

4 participants