-
Notifications
You must be signed in to change notification settings - Fork 27.4k
chore: completely remove the ng scenario runner #15931
Conversation
3887cbe
to
74141de
Compare
src/ngMock/browserTrigger.js
Outdated
* @param {Object=} eventData An optional object which contains additional event data (such as x,y | ||
* coordinates, keys, etc...) that are passed into the event when triggered | ||
* coordinates, keys, etc...) that are passed into the event when | ||
* triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do changes in this file belong in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a little improvement on the formatting, doesn't hurt, right? 😅 Or do you mean browserTrigger docs in general? We moved bT before in preparation of the scenario runner removal, but never documented it,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we be more specific about what properties are supported in eventData
what they are used for?
src/ng/browser.js
Outdated
@@ -67,7 +67,7 @@ function Browser(window, document, $log, $sniffer) { | |||
|
|||
/** | |||
* @private | |||
* Note: this method is used only by scenario runner | |||
* Note: this method is used only by protractor / scenario runner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is protractor really using this? I think it uses testability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testability.whenStable calls this function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, but this comment does not belong here. For example, we have discussed in the past that we should make Testability public, which means that anyone could use whenStable()
. And if we do that, it is very unlikely that someone will remember to update this comment.
If we want to have such a comment, it should be on whenStable()
imo.
src/ngMock/browserTrigger.js
Outdated
* @param {Object=} eventData An optional object which contains additional event data (such as x,y | ||
* coordinates, keys, etc...) that are passed into the event when triggered | ||
* coordinates, keys, etc...) that are passed into the event when | ||
* triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we be more specific about what properties are supported in eventData
what they are used for?
@@ -938,8 +938,6 @@ function $LocationProvider() { | |||
// update location manually | |||
if ($location.absUrl() !== $browser.url()) { | |||
$rootScope.$apply(); | |||
// hack to work around FF6 bug 684208 when scenario runner clicks on links | |||
$window.angular['ff-684208-preventDefault'] = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still used in browserTrigger
: https://github.com/angular/angular.js/pull/15931/files#diff-6fcc18d824a1325cf54ef306cd2fb7a5R124
74141de
to
95400c3
Compare
The fixup is actually wrong, the FF removal is diffeent from the docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the docs :heart:
LGTM (with a couple of nits/suggestions) 👍
src/ngMock/browserTrigger.js
Outdated
* - `y`: y-coordinates for [MouseEvent](https://developer.mozilla.org/docs/Web/API/MouseEvent) | ||
* and [TouchEvent](https://developer.mozilla.org/docs/Web/API/TouchEvent). | ||
* | ||
* - `relatedTarget`: the related target for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also link to MouseEvent.relatedTarget.
src/ngMock/browserTrigger.js
Outdated
* | ||
* - `cancelable`: [Event.cancelable](https://developer.mozilla.org/docs/Web/API/Event/cancelable). | ||
* Not applicable to all events. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing *
😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also mention keyCode
, charCode
and which
for key*
events.
000644c
to
48d1b2e
Compare
The runner has been deprecated and undocumented since 2014: See commit 8d6d126899d4b1927360599403a7592011243270 Closes angular#9405 BREAKING CHANGE: The angular scenario runner end-to-end test framework has been removed from the project and will no longer be available on npm or bower starting with 1.7.0. It was deprecated and removed from the documentation in 2014. Applications that still use it should migrate to [Protractor](http://www.protractortest.org). Technically, it should also be possible to continue using an older version of the scenario runner, as the underlying APIs have not changed. However, we do not guarantee future compatibility.
The bug was fixed in Firefox 48: https://bugzilla.mozilla.org/show_bug.cgi?id=684208, and only affected the scenario runner
48d1b2e
to
23087b5
Compare
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Removes the deprecated scenario runner (protractor ancestor)
Closes #9405
TODO:
Please check if the PR fulfills these requirements
Other information: