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

fix(build): Use Angular's testability API to wait for end of e2e tests #3911

Closed
wants to merge 1 commit into from

Conversation

marclaval
Copy link
Contributor

Fixes #3829

Some comments:

  • benchmarks never need to wait for Angular 2. Instead of doing changes in each of them, the patch is simply not done when running benchmarks
  • when an e2e test doesn't need to wait for a bootstrap, we use the native .get() from webdriver instead of the patched one from Protractor.
  • Testability API is not available for web workers cases, so they are managed as if no waiting was needed

@marclaval marclaval force-pushed the issue3829 branch 2 times, most recently from c230734 to b060532 Compare August 31, 2015 14:45
@marclaval marclaval added the action: review The PR is still awaiting reviews from at least one requested reviewer label Aug 31, 2015
var result = _get.apply(this, arguments);
browser.driver.wait(protractor.until.elementLocated(By.js('var cs = document.body.children; var isLoading = false; for (var i = 0; i < cs.length; i++) {if (cs[i].textContent.indexOf("Loading...") > -1) isLoading = true; } return !isLoading ? document.body.children : null')), sleepInterval);
return result;
// Benchmarks never need to wait for Angular 2 to be ready
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this? Benchmarks also contain regular Ng2 applications...

@tbosch tbosch added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 11, 2015
@tbosch tbosch assigned marclaval and unassigned tbosch Sep 11, 2015
@tbosch
Copy link
Contributor

tbosch commented Sep 28, 2015

@Mlaval ping?

@marclaval marclaval force-pushed the issue3829 branch 3 times, most recently from b23e967 to ec05ecf Compare September 29, 2015 21:17
@marclaval marclaval removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 29, 2015
@marclaval marclaval assigned tbosch and unassigned marclaval Sep 29, 2015
@@ -11,7 +11,8 @@ describe('ng2 static tree benchmark', function() {
url: URL,
buttons: ['#ng2DestroyDom', '#ng2CreateDom'],
id: 'ng2.static.tree.create.viewcache',
params: [{name: 'viewcache', value: 'true'}]
params: [{name: 'viewcache', value: 'true'}],
waitForAngular2: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this the default (waitForAngular2=true) and then not specify it in every benchmark configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@tbosch tbosch added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 30, 2015
@tbosch tbosch assigned marclaval and unassigned tbosch Sep 30, 2015
@marclaval marclaval added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 30, 2015
@marclaval marclaval assigned tbosch and unassigned marclaval Sep 30, 2015
@tbosch tbosch added the action: merge The PR is ready for merge by the caretaker label Oct 1, 2015
@mary-poppins
Copy link

Merging PR #3911 on behalf of @tbosch to branch presubmit-tbosch-pr-3911.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Oct 1, 2015
@marclaval marclaval closed this in 33593cf Oct 1, 2015
@juliemr
Copy link
Member

juliemr commented Oct 1, 2015

FYI - we'll be moving some of this logic in to Protractor in the next release. Leaving myself a note to make sure to modify what needs to be updated here.

@marclaval marclaval deleted the issue3829 branch December 11, 2017 10:07
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: review The PR is still awaiting reviews from at least one requested reviewer cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Angular's testability API to wait for end of e2e tests
5 participants