Skip to content
This repository has been archived by the owner on Aug 5, 2020. It is now read-only.

Refactor test suite to Jest (+watch +coverage) #437

Merged
merged 16 commits into from
Jan 23, 2017
Merged

Refactor test suite to Jest (+watch +coverage) #437

merged 16 commits into from
Jan 23, 2017

Conversation

ferrannp
Copy link
Contributor

@ferrannp ferrannp commented Jan 20, 2017

Related to #406

This is a pull request to get familiar also with Jest and play with it (specially with the --watch mode). If we are happy with it, we could also introduce Jest to the generated app with gluestick so we can get rid of istambul (solving #333), mocha, karma, sinon and expect (maybe even more if we needed special babel deps for it).

Watch mode

watch

Debugging

screen shot 2017-01-20 at 17 53 11

screen shot 2017-01-20 at 17 53 16

@ferrannp ferrannp changed the title Refactor test suite to Jest Refactor test suite to Jest (+watch +coverage) Jan 20, 2017
@sethbattin
Copy link
Contributor

Followon from #322 : my first thought is this: https://youtrack.jetbrains.com/issue/WEB-14979 <- jest does not have direct webstorm support yet. :|

But that's really just ide-sugar anyway. As long as there is a cli entry point and file saving triggers watch mode, then we're good regardless of explicit support. And if debugging via devtools works, then that solves the other use case.

I will try this one out in earnest later in the day.

@zamotany
Copy link
Contributor

Debugging with Chrome, just need to follow these steps: https://facebook.github.io/jest/docs/troubleshooting.html#tests-are-failing-and-you-don-t-know-why

They use node-inspector but we are removing it in favour of node --inspect (#322).

@zamotany
Copy link
Contributor

zamotany commented Jan 20, 2017

https://youtu.be/1cfA87h_1hI
on Ubuntu 16.04

Notice that debugger; statement in test doesn't trigger break.

@toddw toddw changed the base branch from develop to v1 January 20, 2017 22:20
@toddw toddw added this to the 1.0 milestone Jan 20, 2017
@toddw
Copy link
Contributor

toddw commented Jan 21, 2017

This looks really cool!

Looks like there are some issues in the build: https://travis-ci.org/TrueCar/gluestick/builds/193784640#L387

@toddw
Copy link
Contributor

toddw commented Jan 21, 2017

The tests are broken specifically when --watch is passed so it is likely something that just needs to be updated in our tests

@ferrannp
Copy link
Contributor Author

ferrannp commented Jan 23, 2017

All test seem to PASS but for some reason the build gets stuck (not exiting with error or success). I am not sure if those console.error(s) or some throwings that are happening in some tests are the cause. I need to investigate/try more on this.


Edit 1:

Ok, I found that the test that makes Travis to fail is new.test.js, that tests promts the user to a question Do you wish to continue? and this keeps Jest to be hanging. I need to investigate a solution for this.

The other thing that concerns me is that all these tests are actually hitting the disk and creating/removing files. Probably, we want unit tests to mock this behaviour. See the exact example with fs https://facebook.github.io/jest/docs/manual-mocks.html


Edit 2:

Ok I have the build passing: https://travis-ci.org/TrueCar/gluestick/builds/194489121

The problem is that I had to run it manually since it looks like that PRs to TrueCar:v1 are not triggering builds in Travis. Can you check this @toddw? I guess it is just to change travis.yml in develop.

@ferrannp ferrannp changed the base branch from v1 to develop January 23, 2017 14:27
@toddw
Copy link
Contributor

toddw commented Jan 23, 2017

I updated .travis.yml so it should now include the v1 branch

@toddw
Copy link
Contributor

toddw commented Jan 23, 2017

The other thing that concerns me is that all these tests are actually hitting the disk and creating/removing files. Probably, we want unit tests to mock this behaviour.

Yeah, since a major part of the CLI is creating files, it does write and read from disk during tests. I'm sure we could mock this but it might get tricky to validate the CLI is actually doing what it should. We are totally open to any suggestions here

@ferrannp
Copy link
Contributor Author

So build is green :). About hitting the disk, I think that for now is fine. Maybe in the future we want to try something but it does not feel as a bottleneck right now. I think this PR is ready as it is @toddw.

@toddw
Copy link
Contributor

toddw commented Jan 23, 2017

works great! Thanks @ferrannp

I was not able to get Chrome inspector to work yet but that's a different issue.
Related (#406 (comment))

@toddw toddw merged commit a78fa4c into TrueCar:v1 Jan 23, 2017
@ferrannp
Copy link
Contributor Author

@toddw you should be able when #445 is merged.

@ferrannp ferrannp deleted the 406_refactor_test_suite branch January 23, 2017 21:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants