test: add test for synchronous access to blink APIs #14637
Conversation
spec/api-browser-window-spec.js
Outdated
| @@ -1286,6 +1286,21 @@ describe('BrowserWindow module', () => { | |||
| }) | |||
| w.loadFile(path.join(fixtures, 'api', 'preload.html')) | |||
| }) | |||
| it('has access to blink APIs', (done) => { | |||
There was a problem hiding this comment.
Can you move this spec to chromium-spec.js, we have been isolating any Web standard api specific testing in there.
Also the original issue was delay loading of some these apis after the script context was created, so it would be good to rewrite this test such that it populates all apis on the window object in the preload as well as in the load web page, compare that there are no differences in the result. Which would basically ensure #13787 (comment) spec.
Also, since these are not blink specific apis, might be worth rewording as has no difference with features on window object before and after context creation
There was a problem hiding this comment.
Also the original issue was delay loading of some these apis after the script context was created, so it would be good to rewrite this test such that it populates all apis on the window object in the preload as well as in the load web page
Yup this makes sense
Can you move this spec to chromium-spec.js
preload scripts aren't chromium features though? I think it should go in the preload section where it is now
There was a problem hiding this comment.
preload scripts aren't chromium features though? I think it should go in the preload section where it is now
Yeah, I was looking at the features it was testing and decided to go with chromium-spec, sounds good to have it here.
MarshallOfSound
changed the title
MarshallOfSound
changed the title
MarshallOfSound
force-pushed
the
add-preload-spec
branch
from
1cc5aa8
to
ab1da48
Compare
spec/api-browser-window-spec.js
Outdated
| done() | ||
| }) | ||
| w.destroy() | ||
| w = new BrowserWindow({ |
There was a problem hiding this comment.
Please use openTheWindow() from this file to open a new window.
"Global" w should eventually be removed.
const preload = <...>
const w = await openTheWindow({<...>})
w.loadFile(<...>)
const [, test] = await emittedOnce(ipcMain, 'answer')
expect(test).to.be.an('object')
expect(test).to.have.own.property('atPreload').that.is.an('array')
expect(test).to.have.own.property('atLoad').that.is.an('array')
<...>
MarshallOfSound
force-pushed
the
add-preload-spec
branch
from
ab1da48
to
544803f
Compare
spec/api-browser-window-spec.js
Outdated
| }) | ||
| w.loadFile(path.join(fixtures, 'api', 'preload.html')) | ||
| const [, test] = await emittedOnce(ipcMain, 'answer') | ||
| expect(test).to.be.ok('object') |
There was a problem hiding this comment.
It should be .to.be.an('object'), not .to.be.ok.
spec/api-browser-window-spec.js
Outdated
| const [, test] = await emittedOnce(ipcMain, 'answer') | ||
| expect(test).to.be.ok('object') | ||
| expect(test.atPreload).to.be.a('array') | ||
| expect(test.atLoad).to.be.a('array') |
There was a problem hiding this comment.
.an('array') is probably better than a.('array') =)
This test should ensure we catch a regression of #13787
MarshallOfSound
force-pushed
the
add-preload-spec
branch
from
544803f
to
6b57f58
Compare
|
@alexeykuzmin PR updated 👍 |
|
No Release Notes |
This test should ensure we catch a regression of #13787
Notes: no-notes