-
Notifications
You must be signed in to change notification settings - Fork 2.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
WIP [poc] frontend api tests #5654
Conversation
00d55bf
to
e4029b4
Compare
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
e4029b4
to
65019df
Compare
Generally it is easy to setup for development mode:
But integration into Electron and CI is tricky. I like an approach though it would force us also write more client code for our APIs. An example of a test here: https://github.com/theia-ide/theia/pull/5654/files#diff-ca8616a6aeb302a3499225248b5c82bc cc @marechal-p @marcdumais-work @svenefftinge |
Ideally i would like to have something like inversify/InversifyJS#1104 But it is not trivial i would leave it out for first attempt. |
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.
Looking good! I don't get everything 100% straight yet, but I'll definitely look into it next week. The test you added looks really simple already.
}); | ||
|
||
it("should show 'Explorer' and 'SCM'", () => { | ||
assert.ok(false); |
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 imagine that you added this line to see if the test would fail?
Does it work without this line? Right now we can see errors in travis.
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.
right :) but:
- results are duplicated, once reported by headless chrome and second time by backend logging frontend console
- it does not look properly formatted i.e. %d %s are not replaced
describe('files tab', () => { | ||
it('should open/close the files tab', async () => { | ||
shell.leftPanelHandler.activate('files'); | ||
await new Promise(resolve => window.requestAnimationFrame(resolve)); |
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.
In this case, is this related to the missing client APIs you were talking about? Would it be a good idea to make .activate
return a promise for when it is activated?
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.
yes, we should do. We have code already in WidgetOpenHandler
. Need to move it into the shell.
@@ -0,0 +1,113 @@ | |||
// @ts-check | |||
const path = require('path'); |
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.
it should not be here
this approach was discarded since it requires building tests as part of the application, see #6409 (comment) |
not to merge, only to test travis integration