Jest testing #133

didoo · 2018-09-15T13:39:40Z

Issue #125

Why this PR:
This is a huge refactoring of all the tests suite. I am opening the PR as a way to open the discussion about the possibility to switch to Jest (instead of Mocha). The benefits of this switch are:

have a tool that allows us to detect changes not only in the formatters as functions, but also in the templates and in general on the actual generated files/output.
use it as an excuse to review all the tests, extend the coverage and the reliability of the tests, and allow us to refactor the code with more confidence
also, Jest is pretty standard in the community, quite handy using the "watch" mode, and above all more Modern™ :)

Description of changes:

converted all the tests from Mocha to Jest using jest-codemods
updated all the tests declaration to use the Jest API methods + removed any dependency of Chai + fixed some tests that were conceptually wrong
moved some of the files inside the test folder (created subfolders to mimic the "lib" folder + added a "__" prefix to the folders used to contain assets, fixtures, etc)
added snapshot tests for all the formatters

Notes:

the CI tests are failing because in local the Date.now() returns GMT while on the server returns UTC (to investigate how to fix/workaround it)

Leftovers:

update the documentation (!important)
remove from the package.json the dependencies which are not needed anymore
update the npm command for the coverage (is still using mocha, I don't know if/how Istanbul can use Jest)
possibly other things? I am not an expert in testing, sorry, so I am surely missing something

Possible improvements:

Folder structure: do we want to keep all the tests under the __tests__ folder in the root of the project, or move the tests near their corresponding code that is being tested? in that case, where do we store the resources (eg. JSON, assets, config, etc) and where do we write the outputs?
Review all the tests, to see if they actually test what they are saying (in a couple of cases, the tests were wrong) and add the missing ones (at least, create an issue with the list and add them little by little)
Have a "complete" case of properties/tokens declarations (currently in __tests__/__properties) that covers all the possible cases and variants. This will improve not only the tests for the snapshots, but in general for all the test suite.
better naming convention for folders and files (__properties, __assets, etc) and for variables (eg. StyleDictionaryExtended): to be discussed and agreed together

Also, during the development I have moved the files in the "test" folder to have a more similar folder structure to the "lib" folder. In doing that, I have noticed that some modules are not tested:

lib/utils/flattenProperties.js
lib/register/template.js
lib/transform/config.js
lib/transform/object.js
I imagine there will be more. Probably having the tests near the code would help in detecting this.

Small questions:

Do we need to keep the "Copyright 2017 Amazon.com" on the top of the tests? It's just a test, is it really needed in the test files? We don't have similar headers for the JSON files :)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

still not completed: Test Suites: 12 failed, 29 passed, 41 total Tests: 18 failed, 176 passed, 194 total Snapshots: 0 total Time: 1.968s, estimated 2s

…d tests under correct folder

…ed tests under correct folder + removed extra file

…mall changes

Test Suites: 37 passed, 37 total Tests: 191 passed, 191 total Snapshots: 0 total Time: 4.185s, estimated 12s

chazzmoney · 2018-09-18T22:34:36Z

A couple links I was perusing based on this conversation;

https://stackshare.io/stackups/jasmine-vs-jest-vs-mocha
https://medium.com/airbnb-engineering/unlocking-test-performance-migrating-from-mocha-to-jest-2796c508ec50

dbanksdesign · 2018-09-19T19:18:59Z

For the copyright text, I'm pretty sure... I'm not a open-source expert, I've just been told to have that in there. We can't comment in JSON files because you can't have comments in a valid JSON file :/ I'll double check about having that copyright comment in tests and get back to you

dbanksdesign · 2018-09-19T19:28:33Z

And this is an awesome start. I know cleaning up tests is not glamorous, but as you point out we definitely have some holes in our testing and a lot of room for improvement.

didoo · 2018-09-20T21:35:10Z

Cool, so looks like it's a go, no? In that case, can you have a look at the open questions above and share your thoughts and personal preferences? So I can proceed and close the last details (and fix the problem with the mock date that breaks the snapshot tests in CI).

PS: re. the copyright text, no problem at all @dbanksdesign, don't spend time on it. It was just strange to see this kind of text in a test file :)

dbanksdesign · 2018-09-20T22:58:34Z

Unless @chazzmoney has any objections, I'm good to go. I'll take a look more closely and try to answer open questions this weekend.

chazzmoney · 2018-09-27T05:41:59Z

Sorry for the delay @didoo - will take a look at this tomorrow and respond shortly thereafter

chazzmoney · 2018-09-28T16:33:55Z

For the date issue, we could switch the tests to Date.UTC. Or we could use moment.
Definitely want to push into develop, and probably Danny or I should create a jest-testing branch to push this into.
Do you want to do all this work yourself? I think we might need to collaborate on this. We could create that jest-test branch and then all create PRs against it.
I'm not opinionated on the folder structure, but I do think it could be improved. I fully trust you and Danny on making sure it is easy to understand which items have tests / which do not, as well as which tests apply to which pieces.
Same for naming convention for folders and files (__properties, __assets, etc) and for variables (eg. StyleDictionaryExtended)

Also, here is my suggested TODO list for completion, stolen from your original comment:

update the documentation
remove from the package.json the dependencies which are not needed anymore
update the npm command for the coverage (is still using mocha, I don't know if/how Istanbul can use Jest)
Fix incorrect tests (will need to review all existing tests)
Add missing tests (lib/utils/flattenProperties.js, lib/register/template.js, lib/transform/config.js, lib/transform/object.js)
Have a "complete" case of properties/tokens declarations (currently in tests/__properties) that covers all the possible cases and variants. This will improve not only the tests for the snapshots, but in general for all the test suite.

Thank you for putting all this together and being so thoughtful about it!

didoo · 2018-09-28T18:04:07Z

Regarding Date.UTC OK, I'll give it a try (I've seen there is also a .toISOString()). I would use moment.js only as last resort :)
Working together on it would be great 🙌
OK on creating a jest-test branch and then all create PRs against it.
Todo list: I see that there are already a couple of "projects", maybe we could create another one for this task (I see that there are "cards" that can be moved like on a trello board); also I see that you can create "task lists" https://help.github.com/articles/about-task-lists/

(reference: boblauer/MockDate#9)

dbanksdesign

Great start! Just a few comments

dbanksdesign · 2018-09-28T16:06:38Z

__tests__/__properties/font.json

+  "asset": {
+    "font": {
+      "icon": {
+        "value": "./test/__assets/fonts/scapp_icons-regular.ttf"


should this be "./tests/__assets/fonts/scapp_icons-regular.ttf" ?

mmm. not sure exactly what that prop refers to. I can't find tests that use it or that TTF file. do you remember something about this test?

dbanksdesign · 2018-09-28T16:32:31Z

__tests__/buildAllPlatforms.test.js

+    expect(helpers.fileExists('./__tests__/__output/android/colors.xml')).toBeTruthy();
+  });
+
+  it('should work with js config', () => {


This test isn't doing what it is intending to I don't think. We should be extending style dictionary with a js config file, like in the current test file: https://github.com/amzn/style-dictionary/blob/master/test/buildAllPlatforms.js

yes, you're right. I didn't notice it was doing two different tests, one for a JSON and one for a JS config file.

I have fixed it in an upcoming commit, please check if it's ok now.

Looks good to me

Looks good to me!

package.json

__tests__/utils/resolveObject.test.js

dbanksdesign · 2018-09-28T17:48:47Z

__tests__/formats/es6Constants.test.js

+  // });
+
+  describe('es6Constants', () => {
+    it('should be a valid JS file', () => {


Because we are using Jest now instead of Mocha, can we test this format in here without jumping through babel hoops?

It looks like we can: https://github.com/facebook/jest#additional-configuration

I don't know how to do it :)

No worries, I'll add that to the project and take it on

didoo · 2018-09-28T20:45:14Z

@chazzmoney @dbanksdesign I may have found a decent solution to the UTC problem. One of you can please run the test locally on your machine (if you prefer, create another local repo, so you can throw it away after the test) and see if all the tests are passing also for you? Thanks

dbanksdesign · 2018-09-28T20:55:24Z

@didoo For me the snapshot tests are still failing, but what I'm getting is the time is different now:

    -  * Generated on Sat, 01 Jan 2000 00:00:00 GMT
    +  * Generated on Sat, 01 Jan 2000 08:00:00 GMT

dbanksdesign · 2018-09-28T21:11:42Z

Also created a project for all this stuff: https://github.com/amzn/style-dictionary/projects/4

didoo · 2018-09-28T21:46:47Z

what I'm getting is the time is different now

ah, dates and javascript! 😢

dbanksdesign · 2018-10-01T22:44:50Z

I created a new branch on this repo called 'jest' that we can start working from. I can change the branch on this PR and merge it in to the newly created branch so we can get started if you want @didoo. From there we can start tackling the open items in the project and filing PRs against that branch. I'm thinking about opening some of these smaller tasks to hacktoberfest because adding some unit tests should be pretty easy?

didoo · 2018-10-01T23:26:53Z

@dbanksdesign OK on everything you said above.

* Jest testing (#133) * moved all the existing tests to Jest * finalised Jest tests for “utils” removing assert dependency * finalised Jest tests for “register” removing assert dependency + moved tests under correct folder * finalised Jest tests for “transform” removing assert dependency + moved tests under correct folder + removed extra file * updated path for “service” files/folders * removed output folder * updated the paths to ignore in the Jest config in package.json * finalised Jest tests for “clean” removing assert dependency + other small changes * added “__output” to the list of folders ignored by Jest * some tunings + more tests * more tests cleanup * fixed test for exportPlatform * fixed last tests, and now all tests are green! * Added first snapshot tests! Yay! * added mock for dates to avoid failing snapshots tests * updated tests * first attempt to fix the UTC date problem on CI (reference: boblauer/MockDate#9) * second attempt to fix the UTC date problem on CI * removed the TZ=UTC env environment to test if is really needed * updated all the occurrences of new Date in the templates * restored linting before running the tests suite * code style fix * fixed wrong porting of the test for buildAllPlatforms * test(all): Fix for all tests to match the date and remove of mockdate (#148) inspiration jestjs/jest#2234 * test(javascript/es6): Add test for es6 (#149) * test: add registerTemplate (#147) * add tests for transform object (#151) * add tests for transform object * split up complex test in multiple smaller tests * Jest flatten props (#163) * Adding tests for lib/utils/flattenProperties.js (#146) * Adding tests for lib/utils/flattenProperties.js * update to use lodash sortby function * update to use lodash sortby function * Add babel-jest (#173)

* improved error messages for registerTemplate * updated error message * Introduce option to control the generation of the "Do not edit" header (#132) * stage #1 - formats.js * stage #2 - templates * reset changes to template + simplified changes to formats (now the “options” object is assigned to the “file” element) * fixed wrong parameter passed to fileHeader function * updated documentation * updates after PR comments * removing the confusing static-style-guide stuff (#157) * Fixes #72 * handle no command and invalid commands with friendly console output (#156) * Add json5 support (#165) * Removing unnecessary backticks (#172) * Merge Jest Branch (#169) * Jest testing (#133) * moved all the existing tests to Jest * finalised Jest tests for “utils” removing assert dependency * finalised Jest tests for “register” removing assert dependency + moved tests under correct folder * finalised Jest tests for “transform” removing assert dependency + moved tests under correct folder + removed extra file * updated path for “service” files/folders * removed output folder * updated the paths to ignore in the Jest config in package.json * finalised Jest tests for “clean” removing assert dependency + other small changes * added “__output” to the list of folders ignored by Jest * some tunings + more tests * more tests cleanup * fixed test for exportPlatform * fixed last tests, and now all tests are green! * Added first snapshot tests! Yay! * added mock for dates to avoid failing snapshots tests * updated tests * first attempt to fix the UTC date problem on CI (reference: boblauer/MockDate#9) * second attempt to fix the UTC date problem on CI * removed the TZ=UTC env environment to test if is really needed * updated all the occurrences of new Date in the templates * restored linting before running the tests suite * code style fix * fixed wrong porting of the test for buildAllPlatforms * test(all): Fix for all tests to match the date and remove of mockdate (#148) inspiration jestjs/jest#2234 * test(javascript/es6): Add test for es6 (#149) * test: add registerTemplate (#147) * add tests for transform object (#151) * add tests for transform object * split up complex test in multiple smaller tests * Jest flatten props (#163) * Adding tests for lib/utils/flattenProperties.js (#146) * Adding tests for lib/utils/flattenProperties.js * update to use lodash sortby function * update to use lodash sortby function * Add babel-jest (#173) * feat(json-nested): Add JSON nested transform (#167) Added JSON nested transform, Added test for it and Documentation update re #139 * Fix errors and improve error messaging (#158) * updated error messaging. Fixes for issues with references. * adding in didoo's test from #118 * cleanup of terminology * fixed resolveObject to correctly replace multiple references. modified testing suite to reflect new test. * updates per comments by didoo and dbanksdesign * case sensitive, oops. * case sensitive, oops. * minor updates based on PR feedback * merging with develop to ensure we stay synched * removing cli error handling and moving to module * removing per dannys comments * making constants for group errors per Dannys comments * switch to error grouping mindset and naming * switch to error grouping mindset and naming * per danny's comment * fix flush to execute across all groups if called with no group; remove flush on uncaught exceptions to prevent confusion * simplify, simplify, simplify * changed out error naming to message mindset, cleaned out console.log, fixed issues with simplified GroupMessages * sepearate circular reference tests into separate expects * avoid using string so we dont get it confused with String * Deprecating templates (#152) * Displaying a warning when using templates in the config or registerTemplate * Moving built-in templates to formats * Porting over a stragler test (#190) * 2.5.0

@didoo

* improved error messages for registerTemplate * updated error message * Introduce option to control the generation of the "Do not edit" header (#132) * stage #1 - formats.js * stage #2 - templates * reset changes to template + simplified changes to formats (now the “options” object is assigned to the “file” element) * fixed wrong parameter passed to fileHeader function * updated documentation * updates after PR comments * removing the confusing static-style-guide stuff (#157) * Fixes #72 * handle no command and invalid commands with friendly console output (#156) * Add json5 support (#165) * Removing unnecessary backticks (#172) * Merge Jest Branch (#169) * Jest testing (#133) * moved all the existing tests to Jest * finalised Jest tests for “utils” removing assert dependency * finalised Jest tests for “register” removing assert dependency + moved tests under correct folder * finalised Jest tests for “transform” removing assert dependency + moved tests under correct folder + removed extra file * updated path for “service” files/folders * removed output folder * updated the paths to ignore in the Jest config in package.json * finalised Jest tests for “clean” removing assert dependency + other small changes * added “__output” to the list of folders ignored by Jest * some tunings + more tests * more tests cleanup * fixed test for exportPlatform * fixed last tests, and now all tests are green! * Added first snapshot tests! Yay! * added mock for dates to avoid failing snapshots tests * updated tests * first attempt to fix the UTC date problem on CI (reference: boblauer/MockDate#9) * second attempt to fix the UTC date problem on CI * removed the TZ=UTC env environment to test if is really needed * updated all the occurrences of new Date in the templates * restored linting before running the tests suite * code style fix * fixed wrong porting of the test for buildAllPlatforms * test(all): Fix for all tests to match the date and remove of mockdate (#148) inspiration jestjs/jest#2234 * test(javascript/es6): Add test for es6 (#149) * test: add registerTemplate (#147) * add tests for transform object (#151) * add tests for transform object * split up complex test in multiple smaller tests * Jest flatten props (#163) * Adding tests for lib/utils/flattenProperties.js (#146) * Adding tests for lib/utils/flattenProperties.js * update to use lodash sortby function * update to use lodash sortby function * Add babel-jest (#173) * feat(json-nested): Add JSON nested transform (#167) Added JSON nested transform, Added test for it and Documentation update re #139 * Fix errors and improve error messaging (#158) * updated error messaging. Fixes for issues with references. * adding in didoo's test from #118 * cleanup of terminology * fixed resolveObject to correctly replace multiple references. modified testing suite to reflect new test. * updates per comments by didoo and dbanksdesign * case sensitive, oops. * case sensitive, oops. * minor updates based on PR feedback * merging with develop to ensure we stay synched * removing cli error handling and moving to module * removing per dannys comments * making constants for group errors per Dannys comments * switch to error grouping mindset and naming * switch to error grouping mindset and naming * per danny's comment * fix flush to execute across all groups if called with no group; remove flush on uncaught exceptions to prevent confusion * simplify, simplify, simplify * changed out error naming to message mindset, cleaned out console.log, fixed issues with simplified GroupMessages * sepearate circular reference tests into separate expects * avoid using string so we dont get it confused with String * Deprecating templates (#152) * Displaying a warning when using templates in the config or registerTemplate * Moving built-in templates to formats * Porting over a stragler test (#190) * 2.5.0 * Added 'json/flat' format (#192) * Fix: #195 (#196) * updating contributing to reflect the package manager and testing suite correctly (#197) * Add Sass maps formats (#193) * added ‘sass/map-flat’ and ‘sass/map-deep’ formats + updated tests * fixed inconsistend newlines in templates for sass maps * improved recursive processJsonNode function * updated snapshots tests * removed unused function * Better examples (#164) * changed folder structure * removed table in Readme of Basic example (not clear and probably also some cells were wrong) * small update for the Basic example to make it more clear how aliases are referenced * renamed the “npm” example to “npm module” * updated “npm” example to use the same config and properties as the “basic” example * removed license (no sense here) and updated package.json * updated the s3 example making it more similar to other examples and adding some more assets to be uploaded and linked/embedded in tokens * updated logo in main Readme in example folder * updated the Readme for the S3 example * tried to re-organise the “react” folder in two separate folders the web app doesn’t compile * removed spaces from “example” sub-folder * renamed “example” folder to “examples” * removed numbers from “examples” sub-folder names * removed space in sub-folder names * added advanced example on how to use a watcher to auto-rebuild see: #171 * small update to Readme for “auto-rebuild-watcher” * added advanced example on how to have a multi-platform multi-brand suite * added advanced example on how to use custom templates * fixed “watch” npm script declaration * moved packages under “devDependencies” for “custom templates” package * added a comment in an example of the lodash templating syntax * remove invisible characters from Readme * added “clean” npm script call where missing in examples package.json * added .gitignore file where was missing in examples folder * updated the config file for the “npm module” example * added a comment to explain better how the “formatter” function works * updated the “init” command to expose only the possible/meaningful options + updated documentation for the “examples” page * added comment about collecting more examples * updated the Readme for the “examples” folder * updated “version.js” script as per Danny suggestion * added advanced example on how to use custom transforms * updated basic example to use “format” instead of “template” to avoid the alert in console * added advanced example about referencing/aliasing * updated example to show reference to an “object-like” value * removed the advanced examples for react and react native * added a “create react app” example (with Sass) * better config for S3 example * simplified the example for “S3” * re-introduced android + ios in S3 example * added a “assets-base64-embed” example * finalised the “assets-base64-embed” example * updated Readme for “npm” example + fixed the “prepublishOnly” script option (previous one was deprecated) * removed the “create-react-app-sass” example (I’ll add it later in a separate ticket) * updated the documentation * New cut at documentation PR using current develop branch (#198) * New cut at documentation PR using current develop branch * Apply @didoo's suggestions from code review Co-Authored-By: chazzmoney <[email protected]> * updates based on didoo's thoughts * Updating the architecture documentation page (#200) * updates per didoo and dbanks * typo * generation differences * minor fixes and updates * making sure sd init command documentation is correct, for now * updates for clarity around properties and references * fixing up another alias piece * Addressing some comments in architecture diagram (#204) * Final touches on build diagram and architecture (#206) * Final touches on build diagram and architecture * Updating build diagram * Updating build diagram * Configuration doc update * fixing snapshot whitespace issues, discovered actual failing test on merge... * Fixing merge conflict issues * v2.6.0 release (#210)

didoo added 16 commits September 14, 2018 12:11

moved all the existing tests to Jest

ccce11f

still not completed: Test Suites: 12 failed, 29 passed, 41 total Tests: 18 failed, 176 passed, 194 total Snapshots: 0 total Time: 1.968s, estimated 2s

finalised Jest tests for “utils” removing assert dependency

6ca983c

finalised Jest tests for “register” removing assert dependency + move…

b8ca175

…d tests under correct folder

finalised Jest tests for “transform” removing assert dependency + mov…

a6ea89e

…ed tests under correct folder + removed extra file

updated path for “service” files/folders

0d76f8a

removed output folder

145d51b

updated the paths to ignore in the Jest config in package.json

04df754

finalised Jest tests for “clean” removing assert dependency + other s…

14693b0

…mall changes

added “__output” to the list of folders ignored by Jest

cca0487

some tunings + more tests

149982a

more tests cleanup

1682ec0

fixed test for exportPlatform

a3ecf3e

fixed last tests, and now all tests are green!

82ab444

Test Suites: 37 passed, 37 total Tests: 191 passed, 191 total Snapshots: 0 total Time: 4.185s, estimated 12s

Added first snapshot tests! Yay!

cc4eea0

added mock for dates to avoid failing snapshots tests

c73d1d8

updated tests

e42201f

didoo mentioned this pull request Sep 26, 2018

Format (registerFormat) vs Templates (registerTemplate) #126

Closed

didoo mentioned this pull request Sep 27, 2018

Improve the "examples" section #136

Closed

didoo added 4 commits September 28, 2018 19:21

first attempt to fix the UTC date problem on CI

5b86080

(reference: boblauer/MockDate#9)

second attempt to fix the UTC date problem on CI

990f913

removed the TZ=UTC env environment to test if is really needed

5b60aba

updated all the occurrences of new Date in the templates

a0f4cd3

dbanksdesign reviewed Sep 28, 2018

View reviewed changes

didoo added 3 commits September 28, 2018 22:18

restored linting before running the tests suite

2845396

code style fix

9aaaa34

fixed wrong porting of the test for buildAllPlatforms

20ddb54

dbanksdesign changed the base branch from master to jest October 1, 2018 23:32

Merge branch 'jest' into jest-testing

598142c

dbanksdesign merged commit 5bbbab7 into amzn:jest Oct 1, 2018

didoo mentioned this pull request Oct 3, 2018

Add unit tests for es6 format #140

Closed

didoo deleted the jest-testing branch January 19, 2019 18:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jest testing #133

Jest testing #133

didoo commented Sep 15, 2018 •

edited

Loading

chazzmoney commented Sep 18, 2018

dbanksdesign commented Sep 19, 2018

dbanksdesign commented Sep 19, 2018

didoo commented Sep 20, 2018 •

edited

Loading

dbanksdesign commented Sep 20, 2018

chazzmoney commented Sep 27, 2018

chazzmoney commented Sep 28, 2018

didoo commented Sep 28, 2018

dbanksdesign left a comment

dbanksdesign Sep 28, 2018

didoo Sep 28, 2018

dbanksdesign Sep 28, 2018

didoo Sep 28, 2018 •

edited

Loading

dbanksdesign Oct 1, 2018

dbanksdesign Oct 1, 2018

dbanksdesign Sep 28, 2018

dbanksdesign Sep 28, 2018

didoo Sep 28, 2018

dbanksdesign Sep 28, 2018

didoo commented Sep 28, 2018

dbanksdesign commented Sep 28, 2018

dbanksdesign commented Sep 28, 2018

didoo commented Sep 28, 2018

dbanksdesign commented Oct 1, 2018

didoo commented Oct 1, 2018

Jest testing #133

Jest testing #133

Conversation

didoo commented Sep 15, 2018 • edited Loading

chazzmoney commented Sep 18, 2018

dbanksdesign commented Sep 19, 2018

dbanksdesign commented Sep 19, 2018

didoo commented Sep 20, 2018 • edited Loading

dbanksdesign commented Sep 20, 2018

chazzmoney commented Sep 27, 2018

chazzmoney commented Sep 28, 2018

didoo commented Sep 28, 2018

dbanksdesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

didoo Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

didoo commented Sep 28, 2018

dbanksdesign commented Sep 28, 2018

dbanksdesign commented Sep 28, 2018

didoo commented Sep 28, 2018

dbanksdesign commented Oct 1, 2018

didoo commented Oct 1, 2018

didoo commented Sep 15, 2018 •

edited

Loading

didoo commented Sep 20, 2018 •

edited

Loading

didoo Sep 28, 2018 •

edited

Loading