-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Added unit tests for lib/markdown #1678
Added unit tests for lib/markdown #1678
Conversation
browser/lib/consts.js
Outdated
@@ -1,10 +1,9 @@ | |||
const path = require('path') | |||
const fs = require('sander') | |||
const { remote } = require('electron') | |||
const { app } = remote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't extract app
from remote
while running tests, following error throws.
var app = remote.app;
^
TypeError: Cannot read property 'app' of undefined
Any ideas why this happened?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we have to make remote
Mockable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As result, require('electron')
just return the path of electron binary, a string.
I think it's an good idea to mock electron
, so I created a new mock helper in tests/helpers/setup-electron-mock.js
, we can add more mocked function into it if needed.
@@ -92,7 +92,7 @@ | |||
"uuid": "^3.2.1" | |||
}, | |||
"devDependencies": { | |||
"ava": "^0.16.0", | |||
"ava": "^0.25.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upgraded to 0.25.0 for snapshot support
@@ -29,5 +29,5 @@ test('RcParser should return a json object', t => { | |||
}) | |||
|
|||
function filePath (filename) { | |||
return path.join('boostnoterc', filename) | |||
return path.join(`${__dirname}/boostnoterc`, filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
breaking change in ava v0.17.0
… production mode" This reverts commit bbcd674.
|
||
const noop = () => {} | ||
|
||
mock('electron', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added mock electron.
browser/lib/consts.js
Outdated
@@ -1,10 +1,9 @@ | |||
const path = require('path') | |||
const fs = require('sander') | |||
const { remote } = require('electron') | |||
const { app } = remote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As result, require('electron')
just return the path of electron binary, a string.
I think it's an good idea to mock electron
, so I created a new mock helper in tests/helpers/setup-electron-mock.js
, we can add more mocked function into it if needed.
As result, I think it's an good idea to mock The change in |
Great work!! 👍 👍 👍 |
fixed conflicts. |
Added unit tests for the issues in recent releases.