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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests now work on windows #478

Merged
merged 1 commit into from Dec 28, 2017

Conversation

Projects
None yet
3 participants
@Nasicus
Contributor

Nasicus commented Dec 22, 2017

The problem was that when we required a file (e.g. arkjs), we did so with relative paths and forward slashes (/).
Funny enough when you execute npm start it worked on windows, but the unit tests did not.

You always got something like this:

Electron 1.7.9 (Node 7.9.0) AccountController "before each" hook for "returns the user accounts only" FAILED
        Error: Cannot find module 'E:devgitark-desktop  est/../client/app\../../package.json'
            at Module._resolveFilename (module.js:470:15)
            at Function.Module._resolveFilename (E:\dev\git\ark-desktop\node_modules\electron\dist\resources\electron.asar\common\reset-search-paths.js:35:12)
            at Function.Module._load (module.js:418:25)
            at Module.require (module.js:498:17)
            at require (internal/module.js:20:19)
            at window.require (E:/dev/git/ark-desktop/client/app/src/init.js:9:2001)
            at window.require (E:/dev/git/ark-desktop/client/app/src/accounts/account.service.js:9:2001)
            at window.require (E:/dev/git/ark-desktop/client/app/src/accounts/account.controller.js:9:2485)
            at window.require (E:/dev/git/ark-desktop/client/app/src/components/account/account-card.controller.js:9:2001)
            at window.require (E:/dev/git/ark-desktop/client/app/src/components/account/delegate-tab.controller.js:9:2001)
        Error: Declaration Location
            at window.inject.angular.mock.inject (E:/dev/git/ark-desktop/node_modules/angular-mocks/angular-mocks.js:3145:25)
            at Context.<anonymous> (accounts/account.controller.js:152:7)

My changes:

  • use path.resolve(__dirName,...) everywhere
  • if require('path') was used more than once in a file introduce const _path
    • Why the _? Because in some files there wer already local variables called path and I didn't want to have any conflicts with these. I then decided to call this variable everywhere _path for unification (if you have a better / other preferred variable name, let me know)

I tested the tests & the app on Windows 10 and Ubuntu (don't have a Mac).

PS:
馃殌 NO MORE VM's for all Windows users! 馃殌

@fluorine

This comment has been minimized.

Show comment
Hide comment
@fluorine

fluorine Dec 24, 2017

This is exactly what I needed in the right time for my tests. Thanks!

fluorine commented Dec 24, 2017

This is exactly what I needed in the right time for my tests. Thanks!

@luciorubeens luciorubeens merged commit defce52 into ArkEcosystem:master Dec 28, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@luciorubeens

This comment has been minimized.

Show comment
Hide comment
@luciorubeens

luciorubeens Dec 28, 2017

Member

Good job! +5 馃憤

Member

luciorubeens commented Dec 28, 2017

Good job! +5 馃憤

@Nasicus Nasicus deleted the Nasicus:bug/tests-on-windows branch Dec 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment