Migrate test cases into mocha test runner #1545
Conversation
- migrate global assertion patch into mocha custom interface : mocha creates global context per each test suite, simple global patching won't work
| expect(Notification).toBeDefined(); | ||
| expect(typeof Notification).toBe('function'); | ||
| expect(Notification).exist; | ||
| expect(Notification).to.be.a('function'); |
There was a problem hiding this comment.
Wow, I'm sorry you had to go through each of these assertions. Could have used something like https://github.com/crysalead-js/chai-jasmine. Did you try that?
There was a problem hiding this comment.
was aware but didn't tempted quite especially given repo is somewhat inactive state. Updating assertion actually wasn't that hard, just simple replace.
|
Holy crap! Nice work. |
|
|
||
| const diagramFunction = global.asDiagram; | ||
|
|
||
| //mocha creates own global context per each test suite, simple patching to global won't deliver its context into test cases. |
There was a problem hiding this comment.
If mocha is creating it's own global context, should we be adding rxTestScheduler, hot, cold, etc to this context? Or are you doing this already and I'm not seeing it?
There was a problem hiding this comment.
rxTestscheduler is set up in https://github.com/ReactiveX/rxjs/pull/1545/files#diff-044a0fafe3ba4e4178acb8de9fb82252R90. Actually that's interesting question, what I found of mocha's global context is it doesn't allow patching existing interfaces (i.e, it, etcs..) cause its interface constructs it everytime per test suite, as you can see in this implementation. Other global instance seems not being affected. Maybe my word on comment is somewhat misleading that all global contexts are completely ignored..
|
Looks good overall, just left a note to spur discussion about where As far as I'm concerned, |
ship it. I couldn't be happier to get rid of Jasmine.
: Cool. Let me do this in this way then - check in this change first, then refactor custom interface per suggestion. It's bit hard to play with this PR since it contains too many files. :) Thanks for reviewing! |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description: ☕
This PR's effort to track #1460, migrate test runner from
jasminetomocha(https://mochajs.org/) withchaifor assertion suite,sinonfor mocking.PR contains amount of file changes, major changes are below.
mochaas test runnertest-helperinto mocha's custom interface by mocha's designed behavior of having separate global context per test suiteRelated issue (if exists):
#1460
NOTE
: This PR doesn't migrate browser test. Current test environment using mocha with custom interfaces makes bit hard to easier migration of karma test configuration, haven't found simple solution yet.
/cc @staltz for visibility & comment.