-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add wait api #17
Add wait api #17
Conversation
.wait(delay) | ||
.then(() => afterDelayTime = Date.now()) | ||
.sendMessageToBot('have you waited ?', 'i waited some time') | ||
.runTest(); |
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.
you need to actually ensure that there is a ~1000 ms delay difference between the two times for this test to pass correctly.
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 actually completely missed that you had the check integrated. I would update it though to explicitly check the before and after times instead of encoding it in the bot response, but this test is valid
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 forget to update the README.md accordingly. You added in //``` for it as needed, I just don't have an automated doc builder for that portion yet, so its a manual process
src/BotTester.ts
Outdated
* @param delayInMiliseconds time to wait in milliseconds | ||
*/ | ||
public wait(delayInMilliseconds: number): BotTester { | ||
this.testSteps.push(() => Promise.delay(delayInMilliseconds)) |
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.
4 spaces for indentation
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.
and terminate lines in a semicolon
.wait(delay) | ||
.then(() => afterDelayTime = Date.now()) | ||
.sendMessageToBot('have you waited ?', 'i waited some time') | ||
.runTest(); |
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 forget to update the README.md accordingly. You added in //``` for it as needed, I just don't have an automated doc builder for that portion yet, so its a manual process
a37eee0
to
39e1305
Compare
Also: - Add linter to ci
39e1305
to
be02aa2
Compare
@microsoftly I add a |
@bilby91 just update README.md. As for the linter ... many of the expect lines (e.g. expect(collection).to.be.empty) cause a linter error. I need to check to see if there's a plugin for chai, throw in some exhaustive |
Hey, Readme and circleci files are already updated. Maybe you didn't notice because I amended my changes. I like to work with one commit per PR to have a more clear history. Let me know if their is any pending change. |
Ah. Yep. I see it now. I think I overlooked it because the readme looks just like a test. I'll add the circleci changes separately and first to get linting enforced. |
Cool. Let me know and I'll rebase
Martín Fernández
On Wed, Sep 06, 2017 at 22:34 Matthew Schwartz < mailto:Matthew Schwartz <notifications@github.com> > wrote:
a, pre, code, a:link, body { word-wrap: break-word !important; }
Ah. Yep. I see it now. I think I overlooked it because the readme looks just like a test.
I'll add the circleci changes separately and first to get linting enforced.
—
You are receiving this because you modified the open/close state.
Reply to this email directly,
#17 (comment)
, or
https://github.com/notifications/unsubscribe-auth/ACGV9wLkeSpR7YjQRqVkrcFSQ7atxkqpks5sf0gqgaJpZM4PN0rm
.
|
Solves #12