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

Extract expectations #11

Merged
merged 1 commit into from
Sep 5, 2017
Merged

Conversation

bilby91
Copy link
Contributor

@bilby91 bilby91 commented Sep 1, 2017

@microsoftly I extracted the expectation logic. I couldn't integrate with jest because it seems that the run and the expectation are not decoupled. I think this should be a good beginning for integrating new expectation frameworks.

@microsoftly
Copy link
Owner

looks like you're using npm. Let's keep the repo consistent and only use yarn for package management.

@@ -160,7 +143,7 @@ export class ExpectedMessage {
delete expectedResponse.agent;

try {
expectWithDeepMatch(clone).to.deep.match(expectedResponse);
expect(clone).toDeepMatch(expectedResponse);
Copy link
Owner

Choose a reason for hiding this comment

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

To help make this cleaner, lets change this to be toDeeplyInclude and replace the expectedResponseCollectionsAsIMessage loop wtih

expect(expectedResponseCollectionAsIMessage).toDeeplyInclude(expectedResponse)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made some of the changes. How would you handle the processing done to every message in the collection if we remove the forEach block ?

@microsoftly microsoftly merged commit 0f6ca6c into microsoftly:master Sep 5, 2017
@santiagodoldan santiagodoldan deleted the extract-expectations branch October 16, 2017 20:45
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

2 participants