-
Notifications
You must be signed in to change notification settings - Fork 15
set up test framework for client, server and the travis #59
Conversation
Signed-off-by: xufengli <xufengli@uk.ibm.com>
84cf963 to
393696d
Compare
| @@ -0,0 +1,41 @@ | |||
| /* | |||
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'd recommend using a deeper test folder structure here to help you out further down the line:
data/valid
data/valid/cto
data/valid/acl
data/valid/project
data/invalid
etc, etc
Otherwise once you build up a large number of resources, it will be unmaintainable
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 will add these as needed per our chat.
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.
move the files to data/valid/cto and data/valid/acl now and will update others as needed.
client/test/extension.test.ts
Outdated
| assert.equal(-1, [1, 2, 3].indexOf(0)); | ||
|
|
||
| // test validation cto | ||
| test("activate", () => { |
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.
we're not actually testing activate here, we're testing the ability to open a file using the vscode api
client/test/extension.test.ts
Outdated
| return Promise.reject(err); | ||
| }); | ||
|
|
||
| // let serverOptions: ServerOptions = { |
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.
Need to remove comments!
client/test/extension.test.ts
Outdated
| // let serverOptions: ServerOptions = { | ||
| // run: { module: serverModule, transport: TransportKind.ipc } | ||
| // } | ||
| console.log('is activate called?'); |
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.
Need to remove console logging
client/test/extension.test.ts
Outdated
| workspace.openTextDocument(uri).then((document) =>{ | ||
|
|
||
| let text = document.getText(); | ||
| console.log('text = ' + text); |
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.
should not have console logging in the test output, as this will clutter the test file output
server/test/server.spec.ts
Outdated
| // The module 'assert' provides assertion methods from node | ||
| import * as assert from 'assert'; | ||
|
|
||
| // You can import and use all API from the 'vscode' module |
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.
Probably don't want this generic template text either
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.
@nklincoln I have updated files per your comments. can you review it again please?
server/test/server.spec.ts
Outdated
| it("validateCtoModelFile", () => { | ||
|
|
||
| let originatingFileName: string = "./test/data/test.cto"; | ||
| console.log('is server called?'); |
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.
no console logging in delivered test code as it clutters the logs - use asserts
server/test/server.spec.ts
Outdated
|
|
||
|
|
||
| // test validation cto | ||
| it("validateCtoModelFile", () => { |
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.
All mocha tests should follow a specific format if using a 'describe' framework, as shown here: https://mochajs.org/
This is so that the actual test intention is clear to a third party
server/test/server.spec.ts
Outdated
|
|
||
| let originatingFileName: string = "./test/data/test.cto"; | ||
| console.log('is server called?'); | ||
| // server.validateCtoModelFile( originatingFileName); |
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.
should this be commented out?
server/package.json
Outdated
| "nyc": { | ||
| "exclude": [ | ||
| "coverage/**", | ||
| "out/**", |
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.
these don't exist in the vscode repository ... why are they here?
1bf0046 to
9875d77
Compare
c76e979 to
eb9de93
Compare
nklincoln
left a comment
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.
some minor changes required, but looks good 👍
server/src/server.ts
Outdated
| //Note: if the command is launched from the keyboard shortcut or the command palette, | ||
| //we will not have any arguments passed in so cannot get the filename to use in the diagram. | ||
| // create a title from the event info | ||
| let diagramTitle = 'Business Network Definition'; // ensible default |
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.
's' missing in sensible
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.
all updated now
client/test/extension.test.ts
Outdated
|
|
||
| }, (err) => { | ||
| assert.ok(false, `error in OpenTextDocument ${err}`); | ||
| return Promise.reject(err); |
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.
this line is redundant as it will fail the test based on the assert above it
client/test/extension.test.ts
Outdated
|
|
||
| }, (err) => { | ||
| assert.ok(false, `error in OpenTextDocument ${err}`); | ||
| return Promise.reject(err); |
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.
don't need this here as the assert above will fail the test
client/test/extension.test.ts
Outdated
|
|
||
| workspace.openTextDocument(uri).then((err) => { | ||
| assert.ok(false, `error in OpenTextDocument ${err}`); | ||
| return Promise.reject(err); |
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 above
Update the client json script Add Apach 2 copyright to the test files Add one invalid test file Add tslint to check all *.ts format. update minor changes. remove the failued test case. Signed-off-by: xufengli <xufengli@uk.ibm.com>
set up auto-test framework for client, server and travis #58
Signed-off-by: xufengli xufengli@uk.ibm.com