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

refactored the palette.js code #1793

Closed
wants to merge 82 commits into from
Closed

refactored the palette.js code #1793

wants to merge 82 commits into from

Conversation

fakela
Copy link
Contributor

@fakela fakela commented Jun 21, 2019

No description provided.

@fakela
Copy link
Contributor Author

fakela commented Jun 24, 2019

Have fixed the file path error

@walterbender
Copy link
Member

Great that the config is working. Can you please explain how tests/palette.js actually tests the palette code?

@fakela
Copy link
Contributor Author

fakela commented Jun 26, 2019

I renamed the test/palette.js cause am testing a function in isolation . The flow of the test is
I) set completed to false
ii) create promise
iii)Run promise callback which sets completed to true
iv) Expect that completed is true
i used the async/ await as this relieves you from putting asserts in the then and catch block and async functions implicitly returns a promise but then it can also be re written to explicitly return a promise instead

@fakela
Copy link
Contributor Author

fakela commented Jun 27, 2019

karma wasnt seeing the other test so i had to change the config file to test/ *js it failed though

@walterbender
Copy link
Member

I am wondering about the namespace for the methods used in the tests. You use delayExecution() from palette.js in the promise-palette test and delay() from blocks.hjs in the promise-blocks test. Maybe something like:

delayPalette()
delayBlocks()

???

I still don't quite understand how these tests work... I'll read up.

@fakela fakela changed the title wrote test on promises added in palette.js refactored the palette.js code Jul 3, 2019
@fakela
Copy link
Contributor Author

fakela commented Jul 3, 2019

the links to the test
https://jsperf.com/threesss between the code i added before and the refactored code
https://jsperf.com/newss/1 between the old code and the code i added

@fakela fakela closed this Jul 3, 2019
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

Successfully merging this pull request may close these issues.

None yet

2 participants