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

Tests for the generated output files/formats (Jest) #125

Closed
didoo opened this issue Sep 6, 2018 · 9 comments
Closed

Tests for the generated output files/formats (Jest) #125

didoo opened this issue Sep 6, 2018 · 9 comments

Comments

@didoo
Copy link
Contributor

didoo commented Sep 6, 2018

In the last week or so I have submitted a couple of PRs, and in both cases (especially in #121) I was afraid to have broken something without knowing. Of course, there are some tests, but if you look at this code for example:

var input = helpers.fileToJSON('./test/properties/nonPropertyNode.json');
var output = helpers.fileToJSON('./test/output/output.json');
assert.deepEqual(output.color.comment, input.color.comment);
assert.deepEqual(output.color.base.comment, input.color.base.comment);
assert.equal(output.color.base.attributes.comment, input.color.base.attributes.comment);

it's testing that the JSON in input and the resulting object in output have the same properties.
But what happens if during the refactoring I am introducing changes after in the pipeline, in the function that generates the output or in the template files? and this results in the generated files being different than expected? My suspect is that we don't know and we don't have tests in place to make us feel confident that the changes are not introducing regressions (at least this was my impression while working on the PRs).

Any ideas, thoughts, suggestions (or alternatives) about trying Jest (and its snapshots capabilities) to cover also the output/generated files?

@chazzmoney
Copy link
Collaborator

We could solve this by adding format/template tests that would ensure that the formats / templates are being generated as expected. That should give us end-to-end testing as per your suggestion.

Additionally, I like the idea of having one set of tests for generating the properties and another section of tests for generating output from those properties.

@didoo
Copy link
Contributor Author

didoo commented Sep 15, 2018

That's the reason why I implemented #107:

screenshot_1738

As soon as that PR will be merged, we'll be able to have reliable snapshot tests too :)

@didoo didoo mentioned this issue Sep 15, 2018
@chazzmoney
Copy link
Collaborator

We now have a usable structure in place for this. Just need to get all the tests set up the way we want.

@didoo
Copy link
Contributor Author

didoo commented Nov 9, 2018

Maybe we can close this one now?

@chazzmoney
Copy link
Collaborator

@didoo I was trying to decide if that was right. Is this done? Or is it only done once we have tests in place for every format?

@didoo
Copy link
Contributor Author

didoo commented Nov 9, 2018

In my initial comment, I was mainly referring to snapshot testing (which, for me, is the most important part, since SD is made to generate output files :) ). AFAIK we are covering all the formats with snapshots testing via __tests__/fomats/all.tests.js, no?

BTW in that file this test should be split:

it('should return ' + key + ' as a string', () => {

this test is for "should return [test] as a string", there should be another declaration for the snapshot ones (
expect(output).toMatchSnapshot();
). Is it OK if I do a PR tomorrow? Or it causes too many conflicts in the resulting tests with the open branches?

@chazzmoney
Copy link
Collaborator

You can make the branch and PR whenever. If you do it before we merge the sass map and example branches, you'll just have to do a pull / merge somewhere. Up to you whether you want to wait or do it now.

@didoo
Copy link
Contributor Author

didoo commented Nov 12, 2018

OK, I'll add it in my todo list then.

@chazzmoney
Copy link
Collaborator

Complete as of v2.7.0

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

2 participants