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

Refactor router with unit tests #33

Closed
wants to merge 8 commits into from

Conversation

acdbaykal
Copy link
Contributor

Hi @davidyuk,

So this is what I'm suggesting as a solution for unit testing in #28 .

@acdbaykal acdbaykal mentioned this pull request Jan 20, 2018
@davidyuk davidyuk force-pushed the refactor-router-with-unit-tests branch 2 times, most recently from 904bbd1 to c6767d4 Compare January 22, 2018 05:14
Copy link
Member

@davidyuk davidyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use proxyquire in unit tests instead of making Router class injectable?
There are some warning when I run this tests:

$ npm run unit

> aepp-identity@0.0.1 unit /home/denis/projects/aeternity/aepp-identity
> cross-env BABEL_ENV=test karma start test/unit/karma.conf.js --single-run

22 01 2018 15:26:20.906:INFO [karma]: Karma v1.7.1 server started at http://0.0.0.0:9876/
22 01 2018 15:26:20.908:INFO [launcher]: Launching browser PhantomJS with unlimited concurrency
22 01 2018 15:26:20.913:INFO [launcher]: Starting browser PhantomJS
22 01 2018 15:26:21.758:INFO [PhantomJS 2.1.1 (Linux 0.0.0)]: Connected on socket ZKk4Q2lkoJMHa3GDAAAA with id 51498719
PhantomJS 2.1.1 (Linux 0.0.0) ERROR LOG: '[Vue warn]: Error in beforeCreate hook: "TypeError: undefined is not a function (evaluating 'this._router.init(this)')"

(found in <Root>)'
PhantomJS 2.1.1 (Linux 0.0.0) ERROR LOG: TypeError{line: 240921, sourceURL: 'http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81', stack: 'beforeCreate@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:240921:26
callHook@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:90291:25
_init@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:91952:13
VueComponent@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:92124:17
install@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:240266:40
use@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:92077:27
http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:115273:18
__webpack_require__@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:20:34
http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:122501:33
http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:123472:34
__webpack_require__@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:20:34
http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:175880:33
__webpack_require__@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:20:34
webpackContext@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:164969:28
forEach@[native code]
http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:186648:26
__webpack_require__@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:20:34
http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:66:37
global code@http://localhost:9876/base/index.js?1d98a52682aa5211e5ffab806f3c1018016e0b81:67:12'}

  router/index.js
    guarding routes
      routing when route change is requested
        ✓ pushes SETUP path if current route is APP_BROWSER and no keystore is present
        ✓ pushes SETUP path if current route is UNLOCK and no keystore is present
        ✓ pushes UNLOCK path if current route is APP_BROWSER and keystore is present but not unlocked
        ✓ does NOT redirect if current route is SETUP and keystore is present but not unlocked
        ✓ pushes APP_BROWSER path if current route is UNLOCK and keystore is present and unlocked
        ✓ does NOT redirect if current route is SETUP and keystore is present and unlocked
        ✓ does not interfere when current route is APP_BROWSER and keystore is present and unlocked
        ✓ does not interfere when current route is SETUP and no keystore is present
        ✓ does not interfere when current route is UNLOCK and keystore is present but unlocked
        ✓ does not interfere when current route is INTRO
      listening on mutations
        ✓ registers a listener for vuex mutations
        ✓ redirects to UNLOCK path when setKeystore mutation is triggered and keystore is present
        ✓ redirects to SETUP path when setKeystore mutation is triggered, keystore is NOT present and current path is APP_BROWSER
        ✓ redirects to APP_BROWSER path when setUnlocked mutation is triggered and keystore is present and unlocked
        ✓ redirects to UNLOCK path when setUnlocked mutation is triggered and keystore is present but locked

PhantomJS 2.1.1 (Linux 0.0.0): Executed 15 of 15 SUCCESS (0.117 secs / 0.048 secs)
TOTAL: 15 SUCCESS


=============================== Coverage summary ===============================
Statements   : 6.79% ( 47/692 )
Branches     : 4.6% ( 16/348 )
Functions    : 2.73% ( 3/110 )
Lines        : 6.41% ( 42/655 )
================================================================================

Also, I have some proposals in #34.

…-improvements

Improvements to router unit tests
@acdbaykal
Copy link
Contributor Author

acdbaykal commented Jan 22, 2018

Personally I'm not a big fan of magic. It makes it harder to understand whats exactly happening and therefore makes it hard to understand the reasons behind errors.

Edit: Also proxyquire is not supporting Webpack. thlorenz/proxyquire#62

@muxe
Copy link
Contributor

muxe commented Jan 22, 2018

as I see it Denis found a way without using proxyquire which works for me
#28 (comment)

Is this solution acceptable for you @acdbaykal ?

@acdbaykal
Copy link
Contributor Author

Alternative solution for #28 accepted. I'm closing this request.

@acdbaykal acdbaykal closed this Jan 22, 2018
@acdbaykal acdbaykal deleted the refactor-router-with-unit-tests branch January 22, 2018 17:07
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

3 participants