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

expect/assert return different promises #62

Closed
networkerror opened this issue Jul 15, 2014 · 9 comments
Closed

expect/assert return different promises #62

networkerror opened this issue Jul 15, 2014 · 9 comments

Comments

@networkerror
Copy link

I'm using this with Cucumber.js, which requires a callback to be called after each test runs.

We basically want to do this:
expect(someElement.getText()).to.eventually.equal('foo').then(function () { callback(); }, function(failure) { callback(failure); });

We originally used the expect() syntax. However, this appears to return the original promise from protractor (or some variation of it). Since this promise is already resolved, it can't be rejected if the test fails. This has caused caused a lot of flakiness in our tests when they fail. (They tend to just hang.)

Recently we tried the assert() syntax. This appears to return a new promise (unless an error message is specified ??? - in which case it throws an exception). This new promise is rejected if the assertion fails.

Should these do the same thing? Is there a way to force expect() to return a new promise that is resolved or rejected based on the outcome of the expectation?

@domenic
Copy link
Collaborator

domenic commented Jul 16, 2014

.then(function () { callback(); }, function(failure) { callback(failure); });

Or just use .notify(callback)

We originally used the expect() syntax. However, this appears to return the original promise from protractor (or some variation of it)

This should not be the case. Can you produce a repro? Preferably one that does not involve external libraries like protracter?

unless an error message is specified ??? - in which case it throws an exception

This should also not be the case. Again, a repro would be quite helpful.

@networkerror
Copy link
Author

I think I have two issues in here. Should we split them?

Here's what we found on assert's throwing exceptions when error text is supplied.

// This one throws an exception.
assert.eventually.include(promise, expState, 'Custom text!')
  .then(callback);
// This one does not
assert.eventually.include(promise, expState)
  .then(callback);

We tracked this behavior down to these lines of code.
https://github.com/domenic/chai-as-promised/blob/master/lib/chai-as-promised.js#L349-L353

@domenic
Copy link
Collaborator

domenic commented Jul 21, 2014

It does not throw an exception. It passes a rejection handler that throws, which causes the returned promise to be rejected.

What type of promises are you using? Are they Promises/A+ compliant?

@networkerror
Copy link
Author

It's webdriver (protractor).

We're trying to use chai-as-promised with cucumber.js and protractor.

@domenic
Copy link
Collaborator

domenic commented Jul 21, 2014

If that throws an exception, then they are not Promises/A+ promises, and cannot be used with Chai as Promised.

@networkerror
Copy link
Author

Sounds like protractor (web driver) promises are the root of our problems. Not sure how we'll deal with that...

Thanks for the quick responses!

@domenic
Copy link
Collaborator

domenic commented Jul 21, 2014

I wonder if a new major version of Chai as Promised could bundle a promise library with it, and wrap the given promises into real Promises/A+ promises. This would allow it to work with jQuery promises as well. Hmm.

This would probably require removing the UMD wrapper though and just going CommonJS.

@domenic
Copy link
Collaborator

domenic commented Jul 21, 2014

In the meantime, I should (a) test to make sure the problem is as I expected; (b) if so, blacklist webdriver promises like I do jQuery promises.

@domenic
Copy link
Collaborator

domenic commented Jul 24, 2014

Realized I am already tracking this as #58.

@domenic domenic closed this as completed Jul 24, 2014
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

No branches or pull requests

2 participants