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

Issue 2917 #3

Closed
wants to merge 4 commits into from
Closed

Issue 2917 #3

wants to merge 4 commits into from

Conversation

tilmanschweitzer
Copy link

I tried to fix the failing tests in your PR angular#3972

Actually, I'm not sure if my changes to element.ts are correct, because this part of the code is quite complex. Although they prevent exceptions in the expected conditions, they return undefined for all test cases I added.

I also added some test cases to cover the other expected condition methods.

Use `falseIfMissing` to handle errors caused by missing elements in
`isEnabled` check.

Fixes failing test in angular#3972
…itions

Add test cases to reproduce the missing element race conditions possible in
expected condition methods `visibilityOf`, `textToBePresentInElement`,
`textToBePresentInValue` and `elementToBeClickable`.
Add error handling for missing elements to fix race condition test cases.

angular#3972
Add test cases for `isDisplayed`, `isSelected` and `isEnabled`
@tilmanschweitzer
Copy link
Author

@sjelin Should I open the PR directly to the protractor repo?

If you merge my changes to your branch first, the discussion in angular#3972 is not lost.

@tilmanschweitzer
Copy link
Author

The changes until e27c035 worked on CircleCI

https://circleci.com/gh/tilmanpotthof/protractor/7

@tilmanschweitzer
Copy link
Author

a7f700f is failing as described

https://circleci.com/gh/tilmanpotthof/protractor/9

@sjelin
Copy link
Owner

sjelin commented Jan 17, 2017

Hey, thanks for the fix + testcases! Probably the best thing to do is for you to submit this as a PR against protractor directly, then I'll clean it up a bit (just to make it more consistant with other protractor changes) and merge it in

@sjelin sjelin closed this Jan 17, 2017
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.

2 participants