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
aio: e2e fixes #20661
aio: e2e fixes #20661
Conversation
You can preview e8a6872 at https://pr20661-e8a6872.ngbuilds.io/. |
@@ -28,7 +28,7 @@ describe('Api pages', function() { | |||
|
|||
it('should show readonly properties as getters', () => { | |||
const page = new ApiPage('api/common/http/HttpRequest'); | |||
expect(page.getOverview('class').getText()).toContain('get body: T|null'); | |||
expect(page.getOverview('class').getText()).toContain('get body: T | null'); |
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.
In your face, clang-format
👊
@@ -34,11 +34,11 @@ | |||
|
|||
<!-- Google Analytics --> | |||
<script> | |||
// Note this is a customised version of the GA tracking snippet to aid e2e testing | |||
// See the bit between /**/.../**/ |
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 likeliked this comment (and the /**/
markers) 😞
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.
"liked" - :-P
Trouble is that to make this comment make sense, I needed to make it longer;
but then the explanation is in the end of line comments below...
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.
Saying that this is a customized version and that the changes are between /**/
is enough for me. It makes it much easier to (a) know that this is customized and (b) what exactly has been changed. If I need more info on what it does etc, I can look up the commit.
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 am not approving this without my comment and/or markers 😁
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.
OK OK
(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ | ||
(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), | ||
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;/**/i.sessionStorage.__e2e__||/**/m.parentNode.insertBefore(a,m) | ||
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g; | ||
~i.name.indexOf('NG_DEFER_BOOTSTRAP')|| // only load library if not running e2e tests |
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.
Where does NG_DEFER_BOOTSTRAP
come from?
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.
Protractor sets it when running e2e tests :-)
I didn't want to put too much into the comments here.
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 wonder if they will drop it at some point (isn't it AngularJS-specific)?
I guess we can worry about that when it happens 😃
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 understand that it is Angular as well as AngularJS.
e8a6872
to
ba527a0
Compare
You can preview ba527a0 at https://pr20661-ba527a0.ngbuilds.io/. |
// Note this is a customised version of the GA tracking snippet to aid e2e testing | ||
// See the bit between /**/.../**/ | ||
// Note this is a customised version of the GA tracking snippet | ||
// See the comments below for more info |
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.
See here @gkalpak ! Comments.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.