Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

fix(element chaining): make element chaining work when the control fl… #4029

Merged
merged 1 commit into from
Jan 30, 2017

Conversation

sjelin
Copy link
Contributor

@sjelin sjelin commented Jan 28, 2017

…ow is disabled

Also added some tests to spec/ts/noCF/smoke_spec.ts double checking that the control flow is off

@juliemr I know we talked offline and said it'd be best not to mess with getWebElement and have chaining only work on value. But given functions like isElementPresent and equals and count, it seems like there are going to be a lot of edge cases on that route and no clear dividing line on which functions should work and which shouldn't. This implementation is relatively clean, and should be very rigorously tested by our suite. So let me know.

…ow is disabled

Also added some tests to `spec/ts/noCF/smoke_spec.ts` double checking that the control flow is off
@juliemr
Copy link
Member

juliemr commented Jan 28, 2017

This looks good to me and the tests are happy, so I'm good. Can you remind me which test was flaky without this? Is there a way we can force that test to always fail w/o this change, to improve testing?

@sjelin
Copy link
Contributor Author

sjelin commented Jan 30, 2017

@juliemr The old flaky test was "should chain element actions". The new test "should run chained element actions in sequence" should always fail without this change.

@sjelin
Copy link
Contributor Author

sjelin commented Jan 30, 2017

(you may have missed the new test because clang apparently reformatted the whole file)

@juliemr
Copy link
Member

juliemr commented Jan 30, 2017

k, LGTM

@sjelin sjelin merged commit a20c7a7 into angular:master Jan 30, 2017
igniteram pushed a commit to igniteram/protractor that referenced this pull request Feb 21, 2017
…ow is disabled (angular#4029)

Also added some tests to `spec/ts/noCF/smoke_spec.ts` double checking that the control flow is off
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants