Skip to content
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

[ARO] Adding a feature to make tester able to create their own test #81

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

erbizard
Copy link

I was unable to make UT, caus every testing command in the package.json return error.
If you got any tips to start them i'm completly open to write them

@coveralls
Copy link

coveralls commented Jul 24, 2018

Coverage Status

Coverage increased (+0.6%) to 92.948% when pulling 1ee88fe on erbizard:feature/functionTest into 26ee915 on microsoftly:dev.

const functionCollection: Function[] = this.expectedResponseCollection as Function[];
let errorString = '';
let success = false;
functionCollection.forEach((func: Function) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried bluebird Promise.map to allow for promise cases?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but we are in the promise context, the "this.internalExpectation.expect(success, error).toBeTrue();" create an error but test still a success

* Verfy the incoming message with custom test defined by tester
* If the function that tester defined return an error, make the test break
* If the function return anything else, the test is considered as good
* I've tryed to use promise as parameter, but in a promise we change scope, so the assert doesn't work
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] spelling error tryed -> tried

@microsoftly
Copy link
Owner

please add tests to the BotTester.mocha.spec and BotTesterFailure.mocha.spec.

@microsoftly
Copy link
Owner

I think it would be helpful to make a unique case to assert against. For instance

botTester.sendMessageToBot('something', (message) => {
// do things

  return 'hello'
}, 'hello')

so that we know what to assert against. A bool return type could be used, but then we lose the benefit of good messaging on failures.

@erbizard
Copy link
Author

Like I said in my first comment, I'm unable to run your test, and create test without testing them is little bit strange.
λ mocha test/**/*.mocha.spec.ts --require node_modules/ts-node/register
"BotTester\node_modules\ts-node\src\index.ts:307
throw new TSError(formatDiagnostics(diagnosticList, cwd, ts, lineOffset))
^
TSError: ⨯ Unable to compile TypeScript"

maybe for your second comment, I can make a process if function return String, to make a better assert.
But i still think it's good that dev can make their own error for fail the test, cause in some case string is not enough information and some tests doesn't test string (example in the README.md with the even test)

@erbizard erbizard force-pushed the feature/functionTest branch 5 times, most recently from fb63b7f to 9a7064f Compare July 25, 2018 15:07
@microsoftly
Copy link
Owner

If an error is thrown inside a test step, that step will be recorded as an error. If a dev wants to fail in their test, that approach would still work

@microsoftly
Copy link
Owner

I can also assure you that the tests do work and your changes break existing functionality. Check out the CI tests -> https://circleci.com/gh/microsoftly/BotTester/242?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@erbizard
Copy link
Author

My last commit got some bug, you right, but i need to start your CI every time i need to test, cause your tests commands doesn't work, atm i don't know how your ci chain is defined, but on my computer your testing routing didn't lauch, after short look i've tried to make them work by changing your "ts-node": "^3.1.0" version, that got 4 majors version late, but nothing make it work.

For the return I might check soon, but i've not many time.

@erbizard
Copy link
Author

Also, no existing functionality were engaged in all my commit, there's only my dev that break every time

@microsoftly
Copy link
Owner

That error is strange considering it can transpile just fine. Can you show the error stack? What ts-node/tsc version did you pull down?

@microsoftly
Copy link
Owner

You're also correct, I think one of the tests must be flaky

@erbizard
Copy link
Author

erbizard commented Jul 25, 2018

"version": "3.3.0"

$ ./node_modules/mocha/bin/_mocha --require node_modules/ts-node/register test/**/*.mocha.spec.ts

C:\Users\arthur.rouet\Desktop\Angie-bot\BOT_Hello_Eng-master\BotTester\node_modules\ts-node\src\index.ts:307
throw new TSError(formatDiagnostics(diagnosticList, cwd, ts, lineOffset))
^
TSError: ⨯ Unable to compile TypeScript
test\mocha\chai\BotTester.mocha.spec.ts (483,13): Type '{ title: string; type: "this is no the correct type"; }[]' is not assignable to type '(ISubmitAction | IOpenUrlAction | IShowCardAction)[]'.
Type '{ title: string; type: "this is no the correct type"; }' is not assignable to type 'ISubmitAction | IOpenUrlAction | IShowCardAction'.
Type '{ title: string; type: "this is no the correct type"; }' is not assignable to type 'IShowCardAction'.
Property 'card' is missing in type '{ title: string; type: "this is no the correct type"; }'. (2322)
at getOutput (C:\Users\arthur.rouet\Desktop\Angie-bot\BOT_Hello_Eng-master\BotTester\node_modules\ts-node\src\index.ts:307:15)

@erbizard
Copy link
Author

erbizard commented Jul 25, 2018

I've also thought to a upgrade for this, just return an array with [excepted,got] results, to make the assert better on this feature. that can make better than just a string, in case of check on carousel, heroCards ... etc
but that can take me a little time to implement it and test it.

If you are okay with that, i can implement this before the end of the week I think.

@microsoftly
Copy link
Owner

microsoftly commented Jul 25, 2018

Are you on windows? I'd imagine that could be a source of the bug. I've not tested outside of an *nix env

@erbizard
Copy link
Author

Yes I'm on windows

@erbizard
Copy link
Author

erbizard commented Aug 1, 2018

But Unit test isn't a problem actually, I've made them and the coverage is okay. The only issue I can see her is the excepted bool to the assert that i'm going to change. When this going to be done i'll post a comment on this trend so you'll be notified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants